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@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jkeating@xxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |jkeating@xxxxxxxxxx Flag| |fedora-review? --- Comment #5 from Jesse Keating <jkeating@xxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review