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=738556 Richard Shaw <hobbes1069@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@xxxxxxxxx --- Comment #2 from Richard Shaw <hobbes1069@xxxxxxxxx> 2011-09-18 08:59:00 EDT --- I can't sponsor you but I have some suggestions for your spec file. 1. What versions of Fedora/EPEL do you plan on building for? Depending on your answer certain sections or commands my be unnecessary. I see you already excluded "BuildRoot" so you must not be planning on building for EPEL 5. If you don't plan on building for EPEL at all then you can remove the whole "%clean" section. Also, "rm -rf %{buildroot}" is no longer needed in %install. The packaging guidelines[1] don't specify in which Fedora/EPEL version %defattr is no longer necessary, only "pre rpm 4.4". 2. I can't remember where I read it but the use of macros for standard shell commands (rm, make, install, etc.) is discouraged and doesn't aid readability. 3. You don't need the extra "/" between %{buildroot} and the other macros, %{_prefix} %{_bindir}, %{_sharedir}, etc. as it's already in the macros[2]. Hopefully someone will sponsor you soon. Until then it doesn't hurt to practice reviewing other packages[3]. Richard [1] http://fedoraproject.org/wiki/Packaging:Guidelines [2] http://fedoraproject.org/wiki/Packaging:RPMMacros [3] http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group -- 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