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