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=734275 Richard Shaw <hobbes1069@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@xxxxxxxxx --- Comment #2 from Richard Shaw <hobbes1069@xxxxxxxxx> 2011-08-29 21:33:09 EDT --- I make take this one if no one else grabs it, but here's a quick review of the spec file. 1. Your Souce: tag should be the full URL whenever possible. There's a specific guideline[1] for SourceForge. 2. You don't need the "BuildRoot:", "%clean", or "defattr" in "%files" if you're not going to build for EL5 (Redhat, CentOS, Scientific Linux). I can't quite remember at what specific release those became unnecessary, but if you're only building for F14+/EL6+ they're all safe to remove. 3. I don't recall any rule for it, but unless it's needed for successful building, I wouldn't do any file manipulation in %prep. In this specific case depending on how "make install" works I would just mv (rename) the file instead of creating a symbolic link. 4. Speaking of building, Fedora has a cmake macro, "%cmake", that takes care of the most common configuration options. In your case, you would replace the whole line with just "%cmake" Also, unless it creates a problem, smp flags should be set on make to speed up building: make %{?_smp_mflags} One (intended) side-effect of this is that the install location will no longer include the build root. Sometimes you can get away with including it but if program hard coded that path somewhere in a binary or library then it would fail. We'll fix the install location in #5. 5. You should never strip the binaries. That would make the debuginfo sub-package useless. Rpmbuild will take care of that for you. Also, don't use macros for common shell commands (rm, mv, cp, install, etc.) unless you're going to override their default behavior. I'm not sure why SUSE uses them so frequently. Now we fix where the files are "installed" using the DESTDIR environment variable. make DESTDIR=%{buildroot} install That's all I see at the spec file level. Be sure to post new links for the updated spec and SRPM. Since you've officially submitted the package you'll need to bump the release to 2 and update your changelog so anyone who follows behind me and easily see what you've done. Richard [1] http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net -- 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