[Bug 455187] Review Request: erlang-pgsql - Erlang PostgreSQL interface

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=455187





--- Comment #3 from Robert Scheck <redhat-bugzilla at linuxnetz.de>  2008-08-25 15:17:41 EDT ---
(In reply to comment #2)
> * Version field should be 0 instead of 0.0.0
> * Instead of using "svn co" and later removing svn files, you should use "svn
> export"
> * You should add verbatim commands used to produce tarball, not only one (svn
> co) - see this spec, for example:

Uh, it must have been late. Of course, I'll change this, thanks for pointing.

> * There is rev. 691 in svn. Would you mind to update your package?

I can do that, it's just a minor bugfix.

> * Using only checkout date in versionin is insufficient at all. You should use
> svn revision (and date, if you wish) instead.

Sorry, but the MUST is the other way round DATEsvn is a must, while your
DATEsvnREVISION suggestion is optional. So I will stay with the current one.
Keep in mind, that the upstream development is more than slow, I can cound
the commits over the last year on one hand or so; see also:

[20:59:35] < rsc> hrm. My reviewer dislikes the skipping of the revision in
%release, so I'm currently only doing DATEsvn rather DATEsvnREV as he expects.
[21:00:04] < tibbs> Your reviewer is entitled to their opinion, I suppose.
[21:00:49] < tibbs> But the guidelines just say "followed by up to 16 (ASCII)
alphanumeric characters of your choosing".
[21:01:08] < tibbs> If you choose to put nothing or just "svn", that is quite
up to you.
[21:02:33] < tibbs> Our intent was not to legislate anything beyond the
presence of the date.

> * Main package should contain doc/short-desc as well.

The content of doc/short-desc seems useless to me. As far as I read, HOWTO file
contains this content as well, so I didn't add the file and will keep this.

> * To avoid building empty debuginfo-subpackage you should add 
> %define debug_package %{nil}
> at top of your spec-file. See this spec as an example:

Accepted.

> * I'm in doubts of naming scheme for devel-subpackage. Actually we can use
> erlang modules in development w/o sources :). Maybe it would be better to name
> it src instead of devel? Just my thoughts, anyway...

I'm currently only using what the current packages are doing. AFAIK there is
no scheme for erlang until now. If I'm switching, the other packages also have
to do so before.

> * You should use -p switch for "install" command, in order to preserve
> timestamps. Frankly speaking in this case (checkout from VCS) there is not so
> much sense, but lately, when official tarball may be introduced, it would have
> more meaning.

I won't use -p once we've something official, currently seems useless to me.

Are you able to life with the "no"s I've stated above? Please give me a short
note if so. As you maybe understand, I'm not happy with flipping some bits or
lines all the time during a review while it is still discussed or similar. Of
course this only referes to the current points.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.