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=225997
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tomspur@fedoraproject.org AssignedTo|nobody@fedoraproject.org |tomspur@fedoraproject.org Flag| |fedora-review?
--- Comment #3 from Thomas Spura tomspur@fedoraproject.org 2010-07-19 10:23:14 EDT --- Review:
Good: - name ok - %{optflags} are used now - no static libs - no *.la - libs correctly packaged - group ok - BR ok - parallel make
Needswork: - patch does not have an upstream bug or a comment, that it was send to the maintainer - please use INSTALL="install -p", when installing to preserve timestamps - use %{_includedir} in %files - please just use "rm -rf $RPM_BUILD_ROOT" and not "[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT" (That's more common and on recent fedora versions, that could even left out completely. But better let them there, till EPEL also supports them.)
- license missing: The doc is licensed under GFDL so license should be: LGPLv2+ and GFDL And please make a note in the spec file, what is under which license. see: https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licens...