[Bug 442329] Review Request: gnome-lirc-properties - Infrared Remote Controls setup tool

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

Summary: Review Request: gnome-lirc-properties - Infrared Remote Controls setup tool


https://bugzilla.redhat.com/show_bug.cgi?id=442329





------- Additional Comments From bnocera at redhat.com  2008-07-03 11:02 EST -------
(In reply to comment #6)
> formal review:
> 
<snip>
> BAD	rpmlint is silent.
<snip>
> BAD	owns the directories it creates.
<snip>
> BAD	scriptlets present.
<snip>
> BAD	is a GUI app.


> bugs
> * %{_sysconfdir}/dbus-1/system.d/*.conf must be marked as %config

It's not a configuration file, and it shouldn't be marked as %config, otherwise
newer versions of gnome-lirc-properties might not launch the service properly.

> * use %{_datadir}/omf/gnome-lirc-properties (the whole directory) in %files

Done.

> * desktop file not installed or validated
> (https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files + some
> discussion on fedora-{devel,packaging} regarding the use of
> desktop-file-validate), calling desktop-file-validate should be sufficient in
> this case

I'm not the one providing the desktop file, it's an upstream file. I can
certainly add a validate call, but using install is useless, as we're not
providing the desktop file ourselves.

> * the scriptlets need minor updates to fully comform the
> https://fedoraproject.org/wiki/Packaging/ScriptletSnippets

I removed the calls to update-desktop-database, as there's no mime-types in the
desktop file. Rest is fixed.

> additional notes
> * occurrences of the string "g-l-p" could be replaced by %{name}, but this is
> only my opinion

I've replaced it where it matters.

> * call to autoconf can be replaced with autoreconf

I'd rather it be broken when a Makefile.am is updated and I forget to call it.

Updated package:
http://people.redhat.com/bnocera/gnome-lirc-properties/gnome-lirc-properties-0.2.8-2.fc9.src.rpm
http://people.redhat.com/bnocera/gnome-lirc-properties/gnome-lirc-properties.spec

-- 
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, or are watching someone who is.