[Bug 191005] Review Request: glob2 - 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: glob2 - Realtime Strategy game


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





------- Additional Comments From j.w.r.degoede at hhs.nl  2006-07-04 04:41 EST -------
Good to hear you've got some decent hosting now and to see you active in
bugzilla again. I'll do a full review of glob2 now. But please understand that
having just one good package isn't enough to get you sponsored. In order to get
you sponsored you must show a thorough understanding of the packaging
guidelines. There are 2 ways to show this understanding:

1)
Do some reviews of other peoples packages, you can use the review below as a
template. Please put a comment above the review that it is not an official
review as you're not yet a Fedora Extras contributer. For the review guidelines
see: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

2)
Have a couple (I usually require 3) packages submitted which have been reviewed
and are approvable (They cannot be actually approved untill you're sponsored).

Your SRPM btw seems to be corrupt, luckily I managed to extract the patch you
use from it, so I can still do a review.

MUST:
=====
O rpmlint output is:
E: glob2 description-line-too-long Globulation 2 is a realtime strategy game
with a different approach on micromanagement.
E: glob2 description-line-too-long Globulation 2 is a realtime strategy game
with a different approach on micromanagement.
W: glob2 no-version-in-last-changelog
W: glob2-debuginfo no-version-in-last-changelog
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersOfferScreen.cpp
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersOfferScreen.h
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersCrossConnectable.cpp
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersChooseMapScreen.h
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MainMenuScreen.cpp
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersCrossConnectable.h
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MultiplayersChooseMapScreen.cpp
E: glob2-debuginfo script-without-shellbang
/usr/src/debug/glob2-0.8.19/src/MainMenuScreen.h
These must all be fixed.
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
O Could not verify that Sources matches upstream because of corrupt SRPM
* Compiles and builds on FC5-i386
* BR: ok (see below)
* No locales
* No shared libraries
* Not relocatable
* Package does not owns / or requires all dirs (see Must fix below)
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* .desktop file as required and properly installed


MUST fix:
=========
* All rpmlint messages. Hint, to fix these:
  E: glob2-debuginfo script-without-shellbang /usr/src/debug/glob2-0.8.19/src/xx
  do "chmod -x src/*" in %prep
* Completly drop the bogus (already commented) Requires: SDL
* Drop unnescesarry automake14 BR, you do not call autoXXX from the specs so
  this isn't needed. Also using old versions of libs/tools in new packages is
  highly discouraged. I now you explicitly added this for mock, mock used to 
  have automake installed by default and you probably needed to add automake14
  because the package breaks with automake15. With neither installed it should
  build fine though as you don't call autoXXX from the spec file.
* Split the BuildRequires line over multiple lines (you can repeat the 
  BuildRequires: on a next line and add more BR's there).
* Remove INSTALL from %doc, since the users use an rpm to have no need for 
  install instructions.
* unowned dir %{_datadir}/%{name} you can fix this by replacing all the
  %{_datadir}/%{name}/xxxx entries under %files with just one entry containging:
  %{_datadir}/%{name}
* Under %files I see %{_datadir}/pixmaps/glob2-icon-xxx.png, that is not 
  according to the freedesktop.org icon standard, it should go under:
  %{_datadir}/icons/hicolor/32x32/apps
  Where 32x32 is the size of the icon, please do ls /usr/share/icons/hicolor/
  to see the available valid sizes, if the icon doesn't match any pick the 
  closest. Also under these dirs it should be named just glob2.png not
  glob2-icon-size.png. The fact that its an icon and its size our made
  clear by the dir its under.
* Once the icons are in the proper case you must add %post(un) script to update 
  the icon-cache see:
http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d
* Don't forget to fix the .desktop file to contain just:
  Icon=glob2.png instead of Icon=glob2-icon-48x48.png

Should fix:
===========
*Redundant BR: SDL_ttf-devel already requires SDL-devel, please remove SDL-devel
 from the BR-s


-- 
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.