[Bug 555018] Review Request: gnac - An audio converter for GNOME (first package, seeking sponsor)

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

Michael Schwendt <mschwendt at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mschwendt at gmail.com

--- Comment #1 from Michael Schwendt <mschwendt at gmail.com> 2010-01-13 16:34:08 EST ---
Only a brief look: It looks fairly well-packaged. The existence of the several
GConf2 related scriptlet sections is evidence that either you've found them in
the Wiki or have copied them from a package.


> $ rpmlint gnac-0.2.1-1.fc12.src.rpm 
> gnac.src: W: no-cleaning-of-buildroot %install
> gnac.src: W: no-buildroot-tag
> 1 packages and 0 specfiles checked; 0 errors, 2 warnings.

In addition to the removal of the buildroot related stuff as discovered by
rpmlint, you could also drop the %clean section, since in F-11 and newer there
is a default one.


> checking for intltool >= 0.35.0...  found

This was a simple rpmbuild. "BuildRequires: intltool" is missing.

Notice that you could do full scratch-builds in Fedora's build system "koji"
even before your packager account is approved:
http://fedoraproject.org/wiki/PackageMaintainers/Join#Install_the_Client_Tools_.28Koji.29


> configure: WARNING: The 'faac' element was not found.
> This will cause encoding to AAC to fail.

What are the implications with regard to file types which need 3rd party
GStreamer plugins?

The .desktop file also defines several MIME types for file types, which are not
directly supported by Fedora. Theoretically, it would be better to drop
unsupported MIME types and let 3rd party add-on packages supply them together
with the needed GStreamer plugins. E.g. in a gnac-freeworld package.


> Summary: An audio converter for GNOME

It's widely accepted practise to omit leading articles, such as "An" and "A" to
shorten summaries even further:

  Summary: Audio converter for GNOME

That also looks better when displayed during installation.


> Requires: libgnomeui

Why is this needed? There is "BuildRequires: libgnomeui-devel", but the
resulting build does not depend on libgnomeui-2.so.0. So, please adhere to
 http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires
for the explicit dependencies.


> %{_datadir}/icons/hicolor

Better:  %{_datadir}/icons/hicolor/*/*/%{name}.*

| MUST: A package must own all directories that it creates.
| If it does not create a directory that it uses, then it
| should require a package which does create that directory.

http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership

That means the "hicolor-icon-theme" package ought to be the only owner of that
directory. And there is an indirect dependency on it through gtk2. A few other
app packages still own %{_datadir}/icons/hicolor, but that is packaging
mistake.


> %{_datadir}/applications/%{name}.desktop

| MUST: Packages containing GUI applications must include
| a %{name}.desktop file, and that file must be properly
| installed with desktop-file-install in the %install section.

Either that, or "desktop-file-validate" must be used.

Also, there is a MIME types definition in the .desktop file. Hence:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database


* As a hint for the %files section: where you include entire directories
recursively, such as 

  %{_datadir}/%{name}

it's more readable to append a slash like

  %{_datadir}/%{name}/

to make explicit that it's a directory and not a single file.

-- 
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.