[Bug 225130] New: Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

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

Summary: Review Request: smashteroid - Astrosmash Remake
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]

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.

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
OtherBugsDependingO|163776                      |163778
              nThis|                            |
              Alias|                            |smashteroid

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]       |[e-mail]
              Alias|smashteroid                 |

------- Additional Comments From [e-mail]  2007-01-29 12:21 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.

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake

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

------- Additional Comments From [e-mail]  2007-01-29 14:48 EST -------
What the hell?  I was reviewing this package?  Why did you remove my name from
the assigned field?

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake

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

------- Additional Comments From [e-mail]  2007-01-29 14:48 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

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]      |[e-mail]

------- Additional Comments From [e-mail]  2007-01-29 14:49 EST -------
Re-assigning bug to me because I was reviewing it before Jochen so rudely
removed my name from asigned feild.

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Alias|                            |smashteroid

------- Additional Comments From [e-mail]  2007-01-29 14:51 EST -------
Readding alias ... Jochen why did you remove the alias too?

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

------- Additional Comments From [e-mail]  2007-01-29 14:59 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....

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

------- Additional Comments From [e-mail]  2007-01-30 21:49 EST -------
Is it easy to patch this so that high scores go in /var/games/smashteroid?

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

------- Additional Comments From [e-mail]  2007-01-31 03:50 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  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

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|163778                      |163779
              nThis|                            |

------- Additional Comments From [e-mail]  2007-01-31 10:16 EST -------
Looks like all MUST FIX items have been fixed.

APPROVED.

[Bug 225130] Review Request: smashteroid - Astrosmash Remake

[Red Hat Bugzilla]

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: smashteroid - Astrosmash Remake
Alias: smashteroid

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

------- Additional Comments From [e-mail]  2007-02-01 05:02 EST -------
Imported and build, closing.