[Bug 498246] Review Request: towhee - A Monte Carlo molecular simulation code

[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=498246





--- Comment #2 from Jussi Lehtola <jussi.lehtola at iki.fi>  2009-04-29 16:08:06 EDT ---
(In reply to comment #1)
> It's only 'pre-review' since I can't be a sponsor.

But you don't need to be a sponsor to do reviews, just an approved packager
(you need to be sponsorED).

> - You package doesn't build. It seems that a BuildRequire is missing (I suppose
> openmpi-devel )

I copypasted the MPI stuff from another spec file and didn't see that the
openmpi requirement was elsewhere. Forgot to try mock build. Fixed.

> - Instead of using command for adding shebang, you should better use a patch in
> the spec, and give it at upstream !

True. Emailed upstream.

> - I think that "%doc license.gpl Examples/" is not a good thing. Because
> Examples/ contains some executable. Install it with the doc flags modify the
> rights.

No, it is quite standard to ship executable scripts in %doc, as the program
works without them; they're just examples of use.

> - In the file /usr/share/towhee/Forcefields, you remove Makefile but not
> Makefile.am and Makefile.in

Nope,
 find Examples/ -name "Makefile*" -exec rm {} \;
also removes Makefile.*


> - Some macros are available in rpm like %{_cat}, %{_cp} ... for coherence you
> should consider to use them.

On the contrary, it is not considered clean to use macros for these sorts of
things. Macros should only be used when absolutely necessary, not for standard
unix commands as cp, mv, rm, mkdir and so on.

> - Extract from guidelines " Use %global instead of %define, unless you really
> need only locally defined submacros within other macro definitions (a very rare
> case)."  

Good catch, this was also because of cut'n'paste. Fixed.


http://theory.physics.helsinki.fi/~jzlehtol/rpms/towhee.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/towhee-6.2.2-2.fc10.src.rpm

-- 
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.