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@xxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |j.w.r.degoede@xxxxxx OtherBugsDependingO|163776 | nThis| | Flag| |fedora-review? ------- Additional Comments From j.w.r.degoede@xxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review