Bug 222994

One item I see immediately: use the %find_lang macro instead of manually 
including the locale file(s).

Ups. Must have missed this on my checklist (spec is quite old). Sry.

Spec URL: http://www.deadbabylon.de/fedora/extras/koverartist/koverartist.spec
SRPM URL:
http://www.deadbabylon.de/fedora/extras/koverartist/koverartist-0.5-4.fc6.src.rpm

3 more comments:
- "mouseclic" in description should be "mouseclick" I think. Shouldn't it be the
plural "mouseclicks" anyway?

- desktop-file-install doesn't look ok: Category "X-Fedora" is obsolete. Main
category is "AudioVideo", while the changelog says "changed group to
Applications/Publishing".

- Not sure if
%{_datadir}/applications/fedora-koverartist.desktop should be
%{_datadir}/applications/kde/fedora-koverartist.desktop instead

(In reply to comment #3)
> 3 more comments:
> - "mouseclic" in description should be "mouseclick" I think. Shouldn't it be the
> plural "mouseclicks" anyway?

Mhh. Think so. (Note to myself: no more c&p in the descriptions)

> - desktop-file-install doesn't look ok: Category "X-Fedora" is obsolete. Main
> category is "AudioVideo", while the changelog says "changed group to
> Applications/Publishing".

Because I cannot say I've understood the desktop-file-usage completly I've used
a modified entry from the package "kover" (also available in extras - and with
X-Fedora). And also the entry "Group: Applications/Publishing".

I'm not really sure where to put it. The rpm-group "Applications/Multimedia"
doesn't seem the best for me. "Applications/Publishing" could be better (because
it is a  printing program) but the corresponding desktop category* "Office"
would not describe it clearly.

* http://standards.freedesktop.org/menu-spec/latest/apa.html


> - Not sure if
> %{_datadir}/applications/fedora-koverartist.desktop should be
> %{_datadir}/applications/kde/fedora-koverartist.desktop instead

Good question. On my installation only amarok and kaffeine (as single packages)
are in %{_datadir}/applications/kde/ but digikam, kompose, konversation,
kshutdown, and some more use /usr/share/applications/fedora-${name}.desktop

(In reply to comment #4)
> 
> Because I cannot say I've understood the desktop-file-usage completly I've used
> a modified entry from the package "kover" (also available in extras - and with
> X-Fedora).

Most of our packages still have X-Fedora, because it used to be in the wiki.
AFAIK in October packaging committee voted to stop using this category, although
this never has been announced officially. Maybe Rex can give us some more
details on this?

> I'm not really sure where to put it. The rpm-group "Applications/Multimedia"
> doesn't seem the best for me. "Applications/Publishing" could be better (because
> it is a  printing program) but the corresponding desktop category* "Office"
> would not describe it clearly.

I don't care where you put it but changelog and desktop file should match. IMHO
Applications/Publishing is ok.

> On my installation only amarok and kaffeine (as single packages)
> are in %{_datadir}/applications/kde/ but digikam, kompose, konversation,
> kshutdown, and some more use /usr/share/applications/fedora-${name}.desktop

If the program runs fine in Gnome/XFCE too, I suggest %{_datadir}/applications,
but Gnome/XFCE will also find it inside of %{_datadir}/applications/kde.

> this never has been announced officially.

It was announced to fedora-maintainers.

Regardless, you should do you best to follow the guidelines as it is *now*. (:

Re: %{_datadir}/applications vs. %{_datadir}/applications/kde
You shouldn't mix-n-match --vendor and %{_datadir}/applications/kde.  Either use:
--vendor=fedora --dir=%{_datadir}/applications
or
--vendor="" --dir=%{_datadir}/applications/kde
in this case, probably the former.

(In reply to comment #5)
> (In reply to comment #4)
> > 
> 
> I don't care where you put it but changelog and desktop file should match. IMHO
> Applications/Publishing is ok.
> 

The changelog entry belongs to a change in the rpm-group. First I'd put this
into "Groups: User Interface/Desktops" and forgotten to change it. It was not
meant to be a change in the desktop-file.

Next round (vendor is still "fedora" 
in /usr/share/applications/fedora-koverartist.desktop):

Spec URL: http://www.deadbabylon.de/fedora/extras/koverartist/koverartist.spec
SRPM URL: 
http://www.deadbabylon.de/fedora/extras/koverartist/koverartist-0.5-5.fc6.src.rpm

Changelog:
- some cleanup in description
- dropped X-Fedora in desktop-file-install

1.  MUST add/use scriptlets for icons:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda

2.  MUST add scriptlets for mimetypes
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef

3.  SHOULD omit
# Set correct entry for "DocPath" in desktop-file
workaround.  It's a workaround for a harmless rpmlint/desktop-file-install warning

4.  (imo) SHOULD set/revert
Group: Applications/Multimedia
(Group isn't really used anywhere, so it's not a big deal)

5.  SHOULD use
desktop-file-install --vendor "" \
 --dir %{buildroot}%{_datadir}/applications/kde

I'll kick off a mock build, and if it builds/runs ok, then fix at least the MUST
items, and I'll APPROVE this.

mock build succeeded, and runs ok.

Fixed 1,2,3
To 4: Applications/Publishing is also used in kover. So I've taken it from there
(maybe it should be changed in both)
To 5: See comment #5 and comment #6

Spec URL: http://www.deadbabylon.de/fedora/extras/koverartist/koverartist.spec
SRPM URL:
http://www.deadbabylon.de/fedora/extras/koverartist/koverartist-0.5-6.fc6.src.rpm

Changelog:
- run gtk-update-icon-cache on post and postun
- run update-desktop-database on post and postun
- added BR: desktop-file-utils

> To 4: (maybe it should be changed in both)

imo, yes, shouldn't be in that comps group either, but that's just me.

> To 5: See comment #5 and comment #6

not a valid justification.  imo, --vendor=fedora should only be used if 
we(fedora) are generating our own .desktop file (which isn't the case here).

These aren't a blocker tho, I'll leave the final decisions up to you.

APPROVED.

(In reply to comment #12)
> imo, --vendor=fedora should only be used if 
> we(fedora) are generating our own .desktop file (which isn't the case here).

Haven't thought about it this way. I will change it to --vendor "".

> APPROVED.

Thanks!

New Package CVS Request
=======================
Package Name: koverartist
Short Description: Create CD/DVD covers
Owners: fedora
Branches: FC-5 FC-6
InitialCC: