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=474603 --- Comment #7 from Lubomir Rintel <lkundrak@xxxxx> 2009-01-08 12:15:24 EDT --- Just a few notes, most of them not serious: 1.) Is this needed? # We don't use any of this. Deleting it so the debuginfo doesn't pick it up. rm -rf source/Irrlicht/jpeglib source/Irrlicht/zlib I think the debuginfo is generated only from files that have an actual reference in dwarf debugging information in binaries 2.) Preserve timestamps for i in include/*.h doc/upgrade-guide.txt source/Irrlicht/*.cpp source/Irrlicht/*.h source/Irrlicht/libpng/*.c source/Irrlicht/libpng/*.h; do sed -i 's/\r//' $i chmod -x $i done I think sed -i modifies a timestamp. I think you should care about keeping the timestamp at least in doc file, to prevent a multilib conflict between the file in packages of different architectures, but, well, I am not quite sure... 3.) You could improve the legibility of your SPEC file cd source/Irrlicht ... make INSTALL_DIR=%{buildroot}%{_libdir} install cd ../.. This could be just make -C source/Irrlicht INSTALL_DIR=%{buildroot}%{_libdir} install 4.) Please use system libpng. While image handling libraries are frequently prone to security vulnerabilities [1], it's probably not a problem for our irrlicht's embedded one since it only reads trusted images. Still, code duplication is bad, please get libpng fixed at least in rawhide. [1] http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=libpng 5.) The license does not seem to be GPLv2+ Short look at the top of ./include/irrlicht.h gives me an impression this is not exactly GPLv2+. Seems like zlib license. The last paragraph (after 3.) seems to suggest something like an advertising clause for IJG code, but I think it is not applicable for this package since we link against external libjpeg, right? You'll definitely know better. * builds fine in mock * optflags used correctly - license (see above) * rpmlint silent * spec file clean and legible -- 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