[Bug 515034] Review Request: nss-softokn - Cryptographic Module of NSS

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


Jesse Keating <jkeating at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |jkeating at redhat.com
         AssignedTo|nobody at fedoraproject.org    |jkeating at redhat.com
               Flag|                            |fedora-review?




--- Comment #5 from Jesse Keating <jkeating at redhat.com>  2009-08-05 01:03:51 EDT ---
Taking this review at Bob's request.

First, the source is just listed as a tarball, with no URL to verify where it
came from.  Also since the tarball has "stripped" in the name, I'm assuming
that some work is being done to remove content.  How that's done needs to be
listed in the spec for verification purposes.

There are a lot of interesting things being done in the spec with very little
comments to explain what's going on.  I'd suggest being a bit more verbose.

When using install, it's preferred to preserve the timestamp with -p.  Also,
calling mkdir to make dirs, and then install to install files seems odd, when
install can just as easily make dirs too.

You're installing something into the prelink.conf.d/ dir but not Requiring
prelink as far as I can tell.  That needs to be fixed.

Your %files section has you taking ownership of the prelink.conf.d directory. 
prelink and only prelink should own that.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

Is there a particular reason why you use %dir for %{_includedir}/nss3 and then
list a ton of files in it?  Are there files that you don't want to package that
wind up in that directory?

I can't build this to test, because I'm missing nssutil-devel >= 3.12.3.99.3

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