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=539472 --- Comment #2 from Michael Schwendt <mschwendt@xxxxxxxxx> 2009-11-23 08:34:04 EDT --- No full review, just some observations: > rm -rf test/unit > > %{__rm} -rf %{buildroot} Either use %{__rm} or "rm" consistently, but mixing them only raises doubts about whether using the macro %{__rm} is needed at all? > %build > rm -rf test/unit > sed -i -e 's/unit//g' test/Makefile.am > sed -i -e 's/test\/unit\/Makefile//g' configure.ac That's a good example of stuff you ought to add comments to in the spec file. Not only to answer the "Why?" question, but also to confirm what this is supposed to achieve and whether the first sed translation might not kill anything unexpectedly with a future version upgrade. > %files > %defattr (-,root,root,-) > %doc COPYING INSTALL ChangeLog > ... > > %files devel > %defattr (-,root,root,-) > %doc COPYING INSTALL ChangeLog > ... Is it really necessary to duplicate %doc files like that? Especially with Fedora, the -devel package requires the base package anyway. > %{_includedir}/memcache* '*' as in "many/any"? Or as in "I don't care whether any version upgrade might move the API headers from to a location that's different from previous releases? > %{_libdir}/%{name}.so.* Macros, in particular %{name}, are overrated. If you wanted to simply rename this package from "libmemcache" to "libmemcache1" or "compat-libmemcache1", you would need to touch the %files section, too. So, %{_libdir}/libmemcache.so.* would be much more readable. -- 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