Spec URL: http://labs.linuxnetz.de/bugzilla/erlang-pgsql.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/erlang-pgsql-0.0.0- 0.20080709svn.src.rpm Library that gives possibility to Erlang programs to connect PostgreSQL databases by plain tcp and execute simple SQL statements. As there are only a few erlang related packages within Fedora currently, I'm adding the people maintaining such packages there on Cc. Feel free to remove yourself if you feel bothered by me.
I'll review it.
TODO items: * 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: http://cvs.fedoraproject.org/viewvc/rpms/flashrom/devel/flashrom.spec?view=markup * There is rev. 691 in svn. Would you mind to update your package? * Using only checkout date in versionin is insufficient at all. You should use svn revision (and date, if you wish) instead. * Main package should contain doc/short-desc as well. * 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: http://peter.fedorapeople.org/nagios-plugins-check_sip.spec * 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... * 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.
(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.
(In reply to comment #3) > > * 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: Ok. > > * 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. Likewise. > > * 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. I think we should, probably, discuss it via fedora-devel maillist. There are more libraries for interpreted languages coming, there similar issues would arise. > > * 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. Ok. > 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. Erlang packaging guidelines are still missing. Are you interested in creating ones? There is a dedicated SIG for erlang: http://fedoraproject.org/wiki/Erlang but still no guidelines.
(In reply to comment #4) > I think we should, probably, discuss it via fedora-devel maillist. There are > more libraries for interpreted languages coming, there similar issues would > arise. I don't care whether -devel or -src or whatelse, but we should be consistent for all packages. I'll rename to whatever gets defined or discussed, I'm lacking a bit time to drive such a discussion the next time. > Erlang packaging guidelines are still missing. Are you interested in creating > ones? There is a dedicated SIG for erlang: > > http://fedoraproject.org/wiki/Erlang > > but still no guidelines. Sorry, I just need PostgreSQL-Support for ejabberd at work - not more and not less, so I'm maintaining the package, but I'm clueless regarding Erlang and all what has to do with. Updated files are here now (should include everything from yesterday): Spec URL: http://labs.linuxnetz.de/bugzilla/erlang-pgsql.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/erlang-pgsql-0-1.20080825svn.src.rpm
REVIEW: +/- rpmlint is not silent: [petro@Sulaco SPECS]$ rpmlint ../RPMS/ppc/erlang-pgsql-* erlang-pgsql.ppc: E: no-binary erlang-pgsql.ppc: E: only-non-binary-in-usr-lib erlang-pgsql-devel.ppc: W: no-documentation erlang-pgsql-devel.ppc: E: only-non-binary-in-usr-lib 2 packages and 0 specfiles checked; 3 errors, 1 warnings. [petro@Sulaco SPECS]$ However these messages should be safely ignored. + The package is named according to the Package Naming Guidelines . + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package meets the Packaging Guidelines . + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license. + File, containing the text of the license, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package must matches the upstream source [petro@Sulaco SOURCES]$ svn export -r 691 https://svn.process-one.net/ejabberd-modules/pgsql/trunk erlang-pgsql-0.new A erlang-pgsql-0.new A erlang-pgsql-0.new/Emakefile A erlang-pgsql-0.new/build.bat A erlang-pgsql-0.new/doc A erlang-pgsql-0.new/doc/HOWTO A erlang-pgsql-0.new/doc/short-desc A erlang-pgsql-0.new/INSTALL A erlang-pgsql-0.new/src A erlang-pgsql-0.new/src/pgsql_util.erl A erlang-pgsql-0.new/src/pgsql.erl A erlang-pgsql-0.new/src/pgsql_proto.erl A erlang-pgsql-0.new/src/pgsql_tcp.erl A erlang-pgsql-0.new/EPLICENSE A erlang-pgsql-0.new/build.sh A erlang-pgsql-0.new/ebin Exported revision 691. [petro@Sulaco SOURCES]$ tar xfz erlang-pgsql-0.tar.gz [petro@Sulaco SOURCES]$ diff -ru erlang-pgsql-0 erlang-pgsql-0.new/ [petro@Sulaco SOURCES]$ + The package successfully compiles and builds into binary rpms on at least one supported architecture (ppc). + All build dependencies are listed in BuildRequires. - The package must own all directories that it creates. You should add %dir %{_libdir}/erlang/lib/pgsql-%{version}/{ebin,src} also + The package does not contain any duplicate files in the %files listing. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT. + The package consistently uses macros, as described in the macros section of Packaging Guidelines . + The package contains code, or permissable content. + Everything, the package includes as %doc, does not affect the runtime of the application. -/+ Header files are in a -devel package. However they're not a header, actually. Not a blocker, anyway. + The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT. + All filenames in rpm packages are valid UTF-8. Consider adding missing directories to %dir (btw, huge amount of already packaged apps does not list every created directory as %dir, so seems that this is not a critical blocker) and this package is APPROVED.
Peter, I'll check that the missing directories are owned before importing. Thanks for your review. New Package CVS Request ======================= Package Name: erlang-pgsql Short Description: Erlang PostgreSQL interface Owners: robert Branches: EL-4 EL-5 F-8 F-9 InitialCC: Cvsextras Commits: no
cvs done.
Thanks for CVS. Peter, I can't see any missing/orphaned directories, even after installing the package and looking twice. Package: erlang-pgsql-0-1.20080825svn.fc8 Tag: dist-f8-updates-candidate Status: complete Package: erlang-pgsql-0-1.20080825svn.fc9 Tag: dist-f9-updates-candidate Status: complete Package: erlang-pgsql-0-1.20080825svn.fc10 Tag: dist-f10 Status: complete 81 (erlang-pgsql): Build on target fedora-5-epel succeeded.
erlang-pgsql-0-1.20080825svn.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/erlang-pgsql-0-1.20080825svn.fc9
erlang-pgsql-0-1.20080825svn.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/erlang-pgsql-0-1.20080825svn.fc8
erlang-pgsql-0-2.20080825svn.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/erlang-pgsql-0-2.20080825svn.fc8
erlang-pgsql-0-2.20080825svn.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/erlang-pgsql-0-2.20080825svn.fc9
erlang-pgsql-0-2.20080825svn.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
erlang-pgsql-0-2.20080825svn.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.