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=592772 --- Comment #3 from Jason Tibbitts <tibbs@xxxxxxxxxxx> 2010-11-23 13:10:07 EST --- I haven't the hardware to test this, and I suspect that no other review has any either, which would explain why you've had no comments on this ticket. But I'm just trying to clear out the old ones, and this package seems clean and simple enough. Here are some comments. You can remove the 0%{?fedora} > 12 conditional; all supported Fedora releases will meet it. You can also remove mention of %python_sitearch, since you don't use it. (You could remove the whole thing at the top if you only intend to target Fedora and RHEL6. You could then also remove BuildRoot, %clean and the buildroot cleaning in %install.) I don't understand why you test for pam_stack in %prep. You control what's on the buildsystem based on the build dependencies you list, and you should know what each Fedora release supports. You can't test what's on the end-user system at package build time. Your %post and %postun scripts do not conform to those given in http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database I don't understand why you make a point of passing CFLAGS in %build, given that this is a noarch package and the C compiler is not being called. In ssome places you reference %{buildroot} while in others, $RPM_BUILD_ROOT. You need to be consistent. (Personally I prefer the one with fewer characters.) -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review