[Bug 226417] Merge Review: shared-mime-info

[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=226417





--- Comment #3 from Bastien Nocera <bnocera at redhat.com>  2008-12-15 08:03:07 EDT ---
(In reply to comment #1)
> I reviewed this package. It surely needs some work:
> 
> * Can bug #459365 be closed now?

Will be when the latest update is stable.

> * gawk is among the default package set and hence doesn't need to be BR'd.

Fixed.

> * The BR perl-XML-Parser >= 2.31-16 is not used at all and can be removed. Am I
> wrong?

Needed for intltool, I change it to "perl(XML::Parser)" though.

> * rpmlint says:
>    shared-mime-info.x86_64: W: devel-file-in-non-devel-package
> /usr/share/pkgconfig/shared-mime-info.pc
> Is there a valid reason why this file is not in a -devel package?
> Also, from the guidelines: "Packages containing pkgconfig(.pc) files
> must 'Requires: pkgconfig' (for directory ownership and usability)."

It's used for applications to detect where the database is installed. It's not
a development file though.

>    shared-mime-info.x86_64: E: explicit-lib-dependency glib2
>    shared-mime-info.x86_64: E: explicit-lib-dependency libxml2
> I believe that these explicit R's can be dropped since rpmbuild itself picks up
> these dependencies.

Done

> * Group tag is "System Environment/Libraries" but I don't see a library in this
> package.

Changed to SE/Base

> * What is wrong with the locale files in the tarball? (A more detailed
> explanation as a comment within the SPEC file please.)

Done.

> Also, assuming you have a legitimate reason to remove these files, why are you
> BR'ing gettext?

Because it's needed to put the translations in the .xml file.

> * The files ChangeLog, HACKING and most importantly COPYING need to be listed
> under %doc.

Added HACKING and COPYING, not ChangeLog, as it replicates data from NEWS
whilst being much bigger.

> * Macros should be used consistently. If you want to use %{__rm} notation, use
> macros for the other commands as well (%{__cat}, %{__make}, etc.).
> OR do it the other way around. But please stay consistent.

Used the commands directly now.

> * Buildroot should be one of these:
>    %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>    %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>    %{_tmppath}/%{name}-%{version}-%{release}-root

Changed it to the second one.

> * According to the COPYING and test-tree-magic.c files, the license tag should
> be GPLv2+

Done.

> * The main source file (Source0) should be given in full URL.

Done.

> * Parallel make must be supported whenever possible. If it is not supported,
> this should be noted in the SPEC file as a comment.

Done.

> * About the defaults.list: Can't we provide a separate list for KDE users? This
> may need some hacking on the source code.

Because KDE doesn't use that file, and they never expressed interest.

> * See
>    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#mimeinfo
> for correct usage of the snippet. It's interesting to see that the very package
> that this guideline is based on doesn't obey the guideline itself (at least,
> partially).

Added a note as to why it shouldn't ignore errors.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.