[Bug 773419] Review Request: warmux - 2D turn-based artillery game

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

--- Comment #2 from Jiri Popelka <jpopelka at redhat.com> 2012-01-16 07:27:28 EST ---
(In reply to comment #1)
Thanks for outstanding review.

> 1) package renaming - FAIL
> there is missing
> Provides: wormux = %{version}-%{release}
>
> so the package does NOT provide a clean update path, see
> https://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

I know, but actually it DOES, see:
https://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#Do_I_need_to_Provide_my_old_package_names.3F

Anyway, I've added the Provides.


> warmux.x86_64: E: incorrect-fsf-address /usr/share/doc/warmux-11.04.1/COPYING
> 
> - please report upstream (note that it needs to be changed in source files too)
I'll try.

> * MUST: The package MUST successfully compile and build into binary rpms on at
> least one primary architecture.
> 
> - FAIL - this doesn't build in rawhide, due to missing zlib include, see
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3697088
I've added the include, but there's still some other problem that I can't
figure out. I think it could be related to new GCC 4.7 used in rawhide as I
can't see any problem in the code.
http://koji.fedoraproject.org/koji/taskinfo?taskID=3705602

> * MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory.
> 
> - FAIL, warmux puts icon into %{_datadir}/icons/hicolor/32x32/apps/ but it
> doesn't seem that its dependency chain includes anything that would own that
Added
 Requires:       hicolor-icon-theme

> - the desktop file should no longer be prefixed "fedora-", renaming the package
> is a nice opportunity to change the desktop file name
Fixed.

> "For new packages, do not apply a vendor tag to desktop files."
> - fail, the icon should NOT have .png suffix specified
Fixed.

> - hm, shouldn't be that touch done only if the icon cache is to be really
> updated (i.e. when the following condition is met, gtk-update-icon-cache is
> available)?
Fixed according to
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

> - I don't see any reason why -data should require the executables (OTOH, what
> to do with data without the game?)
You actually answered that, i.e. data requires executable because it's useless
without it.

> - btw, does the game work with older data? - shouldn't be the dependency of
> warmux on warmux-data also versioned?
Fixed.

> I do not understand one thing:
> %configure --disable-nls --disable-dependency-tracking
> ... why disable-nls?
Changed to
%configure --enable-fribidi
and using %find_lang macro now.

--- Summary ---
I think I've fixed all the problems you mentioned except the rawhide building
problem which I'll try to narrow down later.
Spec URL: http://jpopelka.fedorapeople.org/warmux.spec
SRPM URL: http://jpopelka.fedorapeople.org/warmux-11.04.1-2.fc16.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.