Fedora Merge Review: cadaver http://cvs.fedora.redhat.com/viewcvs/devel/cadaver/ Initial Owner: jorton
The current package looks already quite good. A few minor possible spec file changes : - Remove the zero epoch from the neon-devel BR - Include a quick comment as to why pie is needed - Change %defattr(-,root,root) to the more usual %defattr(-,root,root,-) (pretty useless, though, since the package doesn't include directories) - Move the %doc line up so that it isn't in between two normal lines I'll test a build and post the results.
Test build went fine, resulting binary is fine. You could remove the obsolete "cadaver-0.22.5.tar.gz.asc" from CVS, though.
Thanks for the review! > - Remove the zero epoch from the neon-devel BR Fixed. > - Include a quick comment as to why pie is needed It's not "needed", it's a feature, I don't think that explaining what -pie is in every spec file which uses it is really a good idea. > - Change %defattr(-,root,root) to the more usual %defattr(-,root,root,-) > (pretty useless, though, since the package doesn't include directories) Fixed. > - Move the %doc line up so that it isn't in between two normal lines And fixed. In -2. Also fixed the use of %makeinstall, which is now unnecessary in 0.23.0.
Good catch, the %makinstall, I've often missed it in my own spec files :-/ Everything looks good, except maybe the NLS : Passing --enable-nls doesn't change anything, and configure always reports "Internationalization: Not built". I'm not sure why since a quick look seems to indicate that the messages about having NLS enabled should lead to "Internationalization" being enabled. Anyway, this isn't vital, and not review specific, so feel free to have a look and close the bug, but I'll fedora-review + anyway.
Ah, the "Not built" thing is a cosmetic configure script bug, I've fixed it upstream. --enable-nls is in fact the default but being explicit here is good since the upstream default has changed over time here. Thanks again for the review.
Package Change Request ====================== Package Name: cadaver New Branches: el6 Owners: stevetraylen See #739496 which was the request for me to take EPEL ownership.
Git done (by process-git-requests).