[Bug 533887] Review Request: raw-thumbnailer - thumbnailer for raw images on gnome

[bugzilla at redhat.com]

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 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |pikachu.2014 at gmail.com
               Flag|                            |fedora-review?

--- Comment #6 from ELMORABITY Mohamed <pikachu.2014 at gmail.com> 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.