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@xxxxxxxxxx> 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review