Bug 225130 - (smashteroid) Review Request: smashteroid - Astrosmash Remake
Review Request: smashteroid - Astrosmash Remake
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-29 03:59 EST by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-01 05:02:24 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2007-01-29 03:59:40 EST
Spec URL: http://people.atrpms.net/~hdegoede/smashteroid.spec
SRPM URL: http://people.atrpms.net/~hdegoede/smashteroid-1.11-1.fc7.src.rpm
Description:
Smashteroid is a remake of the old Intellivision game Astrosmash. Your job is
to defend Earth from the onslaught of asteroids.  Features three game modes
with exciting new twists.
Comment 1 Jochen Schmitt 2007-01-29 12:21:51 EST
Good:
+ License look fine.
+ Tarball matchs with upstream.
+ Nameing seams ok.
+ Local build works fine
+ Gaming is running on my machine.
+ Installing and uninstalling works fine.
+ Build on mock works fine.
+ rpmlint is quite on source and binary package.
+ rpmlint is quite on install package.

Bad:
- Please include license material to the docs.
Comment 2 Christopher Stone 2007-01-29 14:48:15 EST
What the hell?  I was reviewing this package?  Why did you remove my name from
the assigned field?
Comment 3 Christopher Stone 2007-01-29 14:48:53 EST
Here is my review:
==== REVIEW CHECKLIST ====
- rpmlint output clean
- package named according to package naming guidelines
- spec filename matches %{name}
- package meets packaging guidelines
- licensed with open source compatible license
- license field matches actual license
- license text included in %doc
- spec written in American english
- spec legible
- sources match upstream
fbdd2aed12da3f2e4802c629fcdd7979  astro111src.zip
- successfully compiles and builds on FC-6 x86_64
- all build dependencies listed in BR
- no locales
- no shared libraries
- not relocatable
- package owns all directories it creates
X package does not pull in all directories it uses
- no duplicates in %files
- file permissions set properly
- contains proper %clean
- macro usage consistent
- contains code
- no large documentation
- files in %doc do not affect runtime
- no header files or static libraries
- no pkgconfig files
- no library files with suffix
- no devel subpackage required
- no libtool archives
- contains proper .desktop file
- does not own files or directories owned by other packages

==== MUST FIX ====
- Shouldnt this require hicolor-icon-theme?

==== SHOULD FIX ====
- Use http://www.t3-i.com/smashteroid.htm as URL
- inform upstream of 64bit compiler warnings

==== QUESTIONS ====
- patch seems to include license-change.txt but its not included in %doc?
- patch seems littered with ^Ms, not an issue, but difficult to read
Comment 4 Christopher Stone 2007-01-29 14:49:42 EST
Re-assigning bug to me because I was reviewing it before Jochen so rudely
removed my name from asigned feild.
Comment 5 Christopher Stone 2007-01-29 14:51:29 EST
Readding alias ... Jochen why did you remove the alias too?
Comment 6 Christopher Stone 2007-01-29 14:59:01 EST
I'm guessing that what happened was that Jochen got a mid-air collision and
decided to completely ignore it and overwrite my changes to this bug....
Comment 7 Christopher Stone 2007-01-30 21:49:24 EST
Is it easy to patch this so that high scores go in /var/games/smashteroid?
Comment 8 Hans de Goede 2007-01-31 03:50:04 EST
Jochen, Christopher, both thanks for your review.

(In reply to comment #3)
> Here is my review:
> ==== MUST FIX ====
> - Shouldnt this require hicolor-icon-theme?
> 
Fixed

> ==== SHOULD FIX ====
> - Use http://www.t3-i.com/smashteroid.htm as URL
Fixed

> - inform upstream of 64bit compiler warnings
I need to send all my changes upstream, if I get around to it I'll fix all the
warnings (also the non 64 bit ones) before doing so.
> 

> ==== QUESTIONS ====
> - patch seems to include license-change.txt but its not included in %doc?
Yes, a must-fix actually as Jochen has correctly identified.

> - patch seems littered with ^Ms, not an issue, but difficult to read
Thats what you get when you take dos source code and port it to Linux with a
patch :)


(In reply to comment #7)
> Is it easy to patch this so that high scores go in /var/games/smashteroid?

Doable yes, easy unfortunately not. Not worth the work IMHO. A patch is ofcourse
welcome.

Here is a new version:
* Wed Jan 31 2007 Hans de Goede <j.w.r.degoede@hhs.nl> 1.11-2
- Not only create but actually package license-change.txt
- Add Requires: hicolor-icon-theme
- Use: http://www.t3-i.com/smashteroid.htm as URL

Go get it here:
Spec URL: http://people.atrpms.net/~hdegoede/smashteroid.spec
SRPM URL: http://people.atrpms.net/~hdegoede/smashteroid-1.11-2.fc7.src.rpm
Comment 9 Christopher Stone 2007-01-31 10:16:01 EST
Looks like all MUST FIX items have been fixed.

APPROVED.
Comment 10 Hans de Goede 2007-02-01 05:02:24 EST
Imported and build, closing.

Note You need to log in before you can comment on or make changes to this bug.