[Bug 238270] Review Request: widelands - realtime-strategy 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 report.

Summary: Review Request: widelands - realtime-strategy game


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


j.w.r.degoede at hhs.nl changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |j.w.r.degoede at hhs.nl
OtherBugsDependingO|163776                      |
              nThis|                            |
               Flag|                            |fedora-review?




------- Additional Comments From j.w.r.degoede at hhs.nl  2007-05-05 04:38 EST -------
MUST:
=====
* rpmlint output is:
W: widelands-data no-documentation
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel i386
* BR: ok
0 locales not handled properly
* No shared libraries, ldconfig not needed
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code and permissable content
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package not needed
* .desktop file as needed

MUST FIX
========
* You install pics/wl-logo-64.png as /usr/share/pixmaps/widelands.png, however
  installing icons under /usr/share/pixmaps is obsolete, it should go under
  /usr/share/icons/hicolor/64x64/apps
* Add icon update cache code to %post and %postun, see scriptlets page on the
  wiki
* Remove update-desktop-database from %post(un) this is only needed when you
  install a new mimitype
* Remove "Version" and "TryExec" from the .desktop file. Version should be  
  set to the actual package version, not 1.0 since thats kinda hard todo for
  this package and since Version isn't actually used by anything just remove it.
  TryExec isn't needed here.
* Merge -data and main package into one, no need / use for a seperate package
  
Should Fix
==========
* You do %define buildnum 10 and then everywhere were you use it you write:
  build%{buildnum} why not just do: "%define build build10" and use %{build}
  where you now use build%{buildnum}?
* Why define rel, why not just directly enter it in the release field?
* I agree with your assesment made in comment #3 about the locale files being
  to generic named to go into the system dir, however they should still be
  marked %lang XX (just like config files should be marked %config)
* It doesn't seem to properly pickup the system language set with LANG,
  translations do work when you change the language from the menu


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.