Spec Name or Url: ftp://ftp.licr.org/pub/software/unix/hmmer.spec SRPM Name or Url: ftp://ftp.licr.org/pub/software/unix/hmmer-2.3.2-1.src.rpm Description: Profile hidden Markov models (profile HMMs) can be used to do sensitive database searching using statistical descriptions of a sequence family's consensus. HMMER is a freely distributable implementation of profile HMM software for protein sequence analysis. License is GPL. Builds in mock. Rpmlint appears clean.
A few questions first : - Why not use %{version} in the source line? It lowers the eventuality of changing the "Version:" line, forgetting to change the source on too, and ending up with a package that contains the old version by seems like it contains the new one. - Same logic applies to the "%ifarch ppc" part : If you have do this, you'll have less chances of forgetting to update the configure options for either of ppc or non-ppc : %configure \ %ifarch ppc --enable-altivec \ %endif --enable-lfs \ --enable-threads - Shouldn't the "make check" go into a separate %check section? One remark to point out that the DESTDIR patch is somewhat overkill, and it would have been easier to simply use : make install BINDIR=$RPM_BUILD_ROOT%{_bindir} MANDIR=$RPM_BUILD_ROOT%{_mandir} Since the Makefile only uses these two locations. Last, you don't seem to include any of the files from the "tutorial" directory, is that on purpose? Other than that, things look pretty good :-)
> - Why not use %{version} in the source line? Oversight. I simply copied the URL from the homepage... Fixed. > - Same logic applies to the "%ifarch ppc" part Agreed. Fixed. > - Shouldn't the "make check" go into a separate %check section? Yup. I'm still not used to having a %check section... (and vim won't colorize it either). Fixed. > the DESTDIR patch is somewhat overkill Ok. Fixed. > Last, you don't seem to include any of the files from the "tutorial" > directory, is that on purpose? Nope. Oversight. Fixed. Updated stuff available here: ftp://ftp.licr.org/pub/software/unix/hmmer.spec ftp://ftp.licr.org/pub/software/unix/hmmer-2.3.2-2.src.rpm Please re-review, and thanks for your comments :) Cheers, Christian
Seems all fine to me now! I'm just not able to test the actual functionality of the binaries, because I don't know what to do with them, but the package itself is fine, so I'll approve it. Next step is for you to import the package, and once it's built, you can close this bug as NEXTRELEASE ;-)
FYI, you didn't (yet) add the new component to the owners.list file in CVS. Don't forget to do it.
I'm probably in dumb mode, but: - where is this owners.list file ? - where is it explained in the wiki ? Thanks for the review :-) Should I still send an "APPROVED" mail to extras-commit, or is this obsoleted by the new review process ?
The owners.list info is not too easy to find, but here it is: http://fedoraproject.org/wiki/Extras/BugzillaAdmin https://www.redhat.com/archives/fedora-maintainers/2005-July/msg00030.html
There's at least one warning printed by GCC 4 during compilation, which would be worth examining and getting rid of.
(In reply to comment #6) > The owners.list info is not too easy to find, but here it is: > http://fedoraproject.org/wiki/Extras/BugzillaAdmin > https://www.redhat.com/archives/fedora-maintainers/2005-July/msg00030.html Ah, right. Thanks. I see the info is now in the wiki. Great :-)
(In reply to comment #7) > There's at least one warning printed by GCC 4 during compilation, which would be > worth examining and getting rid of. If you mean this one: gsi.c: In function 'GSIWriteIndex': gsi.c:270: warning: comparison is always false due to limited range of data type I had a look before submitting the package, and it seemed harmless, caused by a useless test on a 16-bit value. If not, could you be a bit more precise ? Thanks.
Ah, okay. Sounds like something upstream might want to get rid of.