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=533887 ELMORABITY Mohamed <pikachu.2014@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |pikachu.2014@xxxxxxxxx Flag| |fedora-review? --- Comment #6 from ELMORABITY Mohamed <pikachu.2014@xxxxxxxxx> 2010-01-28 23:28:26 EST --- Looks better :-). I will review your package (I'm now sponsored ^^). rpmlint is silent, and mock builds fine the package for F11, F12 and rawhide. Some (little) remarks and changes I suggest you: * according to the comments in the source code, the License tag should be « GPLv2+ »; * your BuildRequires seem all OK (no useless BR nor missing one according to configure.ac in the sources, mock builds fine your package); anyway you should replace « perl-XML-Parser » by « perl(XML::Parser) », as recommanded for Perl modules called as Requires/BuildRequires. * I suggested in my comment #2 to add the option « --disable-schemas-install » in your %configure. Thanks to this option, you'll don't need the line « export GCONF_DISABLE_MAKEFILE_SCHEMA_INSTALL=1 » in your %install section. Upstream developers handle correctly this option (see in sources data/Makefile.am), let's use it to honor them ;) * the scriptlets are now OK for the GConf files installation. Anyway you should leave the « %{name}.schemas » template in the scriptlets: replace all the occurences of « %{_sysconfdir}/gconf/schemas/raw-thumbnailer.schemas » with « %{_sysconfdir}/gconf/schemas/%{name}.schemas » (dont forget also in %files). This is a good practice to use macros wherever it's possible to make easier the maintenance. * GConf files should not have the « %config(noreplace) » tag in %files, « %config » is enough (whatever rpmlint says, it's a false positive for GConf files ;-) ) * there's a README file in the sources, but it is empty, so it's OK not to add it in %doc :) Although it's a very little detail, maybe upstream should do something for this file (fill it or remove it from sources ^^). -- 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