[Bug 478399] Review Request: ircp-tray - Infrared file transfer applet for GNOME panel

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





--- Comment #3 from Lubomir Rintel <lkundrak at v3.sk>  2008-12-29 13:24:24 EDT ---
(In reply to comment #2)
> formal review is here, see the notes below:
> BAD BuildRequires are proper.
> BAD final provides and requires look sane.
> BAD no duplicates in %files.
> BAD GUI app with desktop file.

> - no need to have pkgconfig as BR: , it is resolved from all used -devel BRs,

Well, did not use to be the case; but this won't go to EPEL anyway since RHEL
lacks IrDA stack, so I removed the explicit pkgconfig dependency in the new
package.

> gtk2-devel is a dependency of libnotify-devel, so the BRs should be only
> "intltool gettext libnotify-devel openobex-devel"

Again -- it did not used to be. Also, I'm wondering if the redundant
buildrequires are forbidden? It makes a lot more sense to me to depend on
libnotify explicitly no matter if gtk does, since it's not guaranteed for
non-essential dependencies to go away (though it is not likely in this case).

> - what is the reason for using a file as Requires?
> (https://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies)

Well, the original reasoning behind that was that I though openobex-apps
package is mis-named (it's far more common to either call it -utils, or place
executables in the main package and put the libraries into -libs package), and
I was afraid it can change. I did not file a bug report for that, and I don't
really care, so I changed the require to openobex-apps.

> - the desktop file is listed twice in %files

Oops, must have been a typo.

> - upstream desktop file installed, but not validated
> (https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage)

Good catch. Thanks!

SPEC: http://netbsd.sk/~lkundrak/SPECS/ircp-tray.spec
SRPM: http://netbsd.sk/~lkundrak/SRPMS/ircp-tray-0.7.3-2.el5.src.rpm

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