[Bug 985065] Review Request: peg-solitaire - Board game played with pegs

[bugzilla at redhat.com]

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

--- Comment #2 from Mario Blättermann <mario.blaettermann at gmail.com> ---
(In reply to Christopher Meng from comment #1)
> Hi,
> 
> 1. I think your desktop file is incorrect.
> 
> [Desktop Entry]
> Exec='/usr/games/peg-solitaire'
> 
> The path is wrong.
> 
> Icon=/usr/share/pixmaps/peg-solitaire.xpm
> 
> This kind of way to define a icon is not recommended now, is it possible to
> change to "Icon=peg-solitaire"?(I'm sorry I haven't tried it)
> 
> MimeType=
> 
> Well this is just a game and no new MIME types are being introduced, so you
> don't need to add %post{un} scriptlet to update the database.
>
First you should have a look at the patch. It makes sure that the desktop file
becomes valid and the executable binary goes into /usr/bin instead of
/usr/games. You refer to that file in the tarball. This is not usable, indeed.

Moreover, the scriptlet doesn't contain anything about mime types. It's only
for updating the desktop database.

> 2. In your spec I can find find_lang script, but you've commented out them,
> and there are language files in the tarball in fact. Please fix.
> 
Please read the comment. The find_lang macro is useful and applicable when
gettext (or even the qt translation chain) places the language files in
/usr/share/locale/language_code/LC_MESSAGES/*. This is not the case,
peg-solitaire behaves completely different from other packages. I was wondering
if it is possible to get find_lang working here, or if it even makes sense to
use it when all the language files are in the same folder anyway.

Well, if I'm forced to use it, any ideas would be welcome.

> 3. I can see "${RPM_BUILD_ROOT}" in your spec.
> 
> However as we can see from the guideline
> 
> http://fedoraproject.org/wiki/Packaging:Guidelines#UsingBuildRootOptFlags
> 
> You can use $RPM_BUILD_ROOT or %{buildroot} to define the buildroot, but now
> you use a special style "${RPM_BUILD_ROOT}"...
> 
> So, if you want to use macro style, change it to %{buildroot}; Otherwise
> please change it to variable style. Mix using 2 style is WRONG, and you mix
> them in one new style is WRONG again.

Fixed.

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/peg-solitaire.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/peg-solitaire-2.0-2.fc19.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=lEMLXCwhDg&a=cc_unsubscribe