[Bug 468554] Review Request: barrage - Kill and destroy as many targets as possible within 3 minutes

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





--- Comment #3 from Christoph Wickert <fedora at christoph-wickert.de>  2008-10-26 07:15:36 EDT ---
(In reply to comment #2)
> Remove unusualy Categories in the desktopfile install like 
> (http://standards.freedesktop.org/menu-spec/latest/apa.html)
> 
> desktop-file-install \
>                     --remove-category="X-Red-Hat-Base;Application" \
>                     --delete-original \
>                     --dir=%{buildroot}%{_datadir}/applications \
> $RPM_BUILD_ROOT/%{_datadir}/applications/%name.desktop

"Application" needs to be removed for it is no longer a valid category.
"X-Red-Hat-Base" can remain. Categories prefixed with X- are considered as
custom categories. Nevertheless "X-Red-Hat-Base" is not really needed anymore,
it was used to define what apps should appear at top level once.

> echo Icon=barrage >> barrage.desktop
> use macros, because it looks better: 
> echo Icon=%{name} >> %{name}.desktop 

Using macros here hardly adds any value. The guidelines say one should use
macros consistently, this means not to mix %{buildroot} and $RPM_BUILD_ROOT for
example, but this does not mean one necessarily needs to use macros everywhere.

Regarding the icon: 48x48 would be better than 32x32.

> in my opinion it is better to write a little patch to correct the categories
> and the missing icon of this desktopfile and
> just validate the desktopfile install. It's easier to handle and minimize and
> clear the spec-file.

I disagree. IMHO a patch should be avoided here, because it adds additional
overhead. desktop-file-install is there for editing and installing desktop
files and will also validate them. 

> I would rather write:
> %{_datadir}/%{name}/

correct

> %doc AUTHORS BUGS ChangeLog COPYING INSTALL README

Remove INSTALL from %doc, it is not needed when installing from rpm and thus
only adds confusion.

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