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=492091 --- Comment #19 from Toshio Ernie Kuratomi <a.badger@xxxxxxxxx> 2009-07-15 14:00:08 EDT --- Here's the complete review. It has a few things we can work on while waiting for the licensing to be resolved:: GOOD: * Package named according to the Guidelines * spec file also named well. * spec file is legible and in American English * Builds on x86 * Builds in koji * No locale files * Not relocatable * Not a dynamic library package * Owns all files and directories it creates and nothing belonging to others * Permissions set properly * Proper %clean * Cleans the buildroot at the beginning of %install NEEDSWORK: * Sources don't match upstream. However, it looks like the only difference is the zip in the SRPM is from an svn checkout (without expnsion of keywords) and the zip at http://code.zikula.org/content/downloads/3 has keywords expanded. Please use the tarball we can retrieve for the official package so others can audit where it comes from if necessary. * Licensing: pnincludes/lightboxXL/js/lightboxXL.js: Creative Commons license. Creative commons licenses are not compatible with the GPL. Since the rest of this plugin is GPL+, this code cannot be used with it. :-( * License tag should be GPL+. Unfortunately, I could find no reference to a specific version of the GPL in any header files. The generic GPLv2 text that's in the license.txt file allows people to use any version of the GPL (GPL+) unless we can find intent that a specific subset is meant (for instance, a header that says GPLv2+ GPL version 2 or later, etc). Often, the code authors don't know this so it's polite to let them know that they should specify what version(s) of the GPL they want somewhere. * %post scriptlet isn't needed. The symlinking should be done in the %install step. Also, should use the filesystem path macros here too. For example, change this:: %post ln -sf /usr/share/php/php-simplepie /usr/share/%{base}/modules/%{module}/pnincludes/simplepie into this:: %install [...] ln -sf %{_datadir}/php/php-simplepie %{buildroot}%{_datadir}/%{base}/modules/%{module}/pnincludes/simplepie - For the pndocs symlink.... I don't think that's needed at all. * Standardise on either %{buildroot} or $RPM_BUILD_ROOT. These two things mean the same thing but using both just gets people confused. I think it got you confused here:: find ${buildroot} -empty -exec rm -f {} \; find ${RPM_BUILD_ROOT} -empty -exec rm -f {} \; You probably just want this:: find %{buildroot} -empty -exec rm -f {} \; (Notice the % instead of the $) * rpmlint: zikula-module-Content.noarch: W: dangerous-command-in-%post ln 2 packages and 0 specfiles checked; 0 errors, 1 warnings - See the earlier note about the %post section. TO FIGURE OUT LATER: * There are language files in pnlang but they aren't gettext locale files. We don't have tooling to detect these and mark them as belonging to a certain language but we probably should find some way to support that in the future.. -- 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