Bug 447639 - Review Request: typespeed - Test your typing speed and get your fingers' CPS
Review Request: typespeed - Test your typing speed and get your fingers' CPS
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-20 18:03 EDT by Michel Alexandre Salim
Modified: 2008-07-31 21:49 EDT (History)
3 users (show)

See Also:
Fixed In Version: 0.6.4-2.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-31 21:49:06 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Michel Alexandre Salim 2008-05-20 18:03:16 EDT
Spec URL: http://salimma.fedorapeople.org/for_review/games/typespeed.spec
SRPM URL: http://salimma.fedorapeople.org/for_review/games/typespeed-0.6.4-1.fc9.src.rpm
Description:
Typespeed gives your fingers' cps (total and correct), typoratio and
some points to compare with your friends.

Typespeed's idea is ripped from ztspeed (a dos game made by Zorlim).
Idea of the game should be clear to anyone, just type and type it fast
or be a loser.
Comment 1 Hans de Goede 2008-05-31 04:29:28 EDT
Wanna swap reviews? :

* elice - Elice is a PureBasic to c++ translator / compiler
https://bugzilla.redhat.com/show_bug.cgi?id=448310

* lostlabyrinth - Lost Labyrinth is a coffeebreak dungeon crawling game
https://bugzilla.redhat.com/show_bug.cgi?id=448311

* lostlabyrinth-sounds - Lost Labyrinth sounds
https://bugzilla.redhat.com/show_bug.cgi?id=448312

* lostlabyrinth-graphics - Lost Labyrinth graphics
https://bugzilla.redhat.com/show_bug.cgi?id=448313

* trophy - Car racing game with special features
https://bugzilla.redhat.com/show_bug.cgi?id=448422
Comment 2 Christoph Wickert 2008-06-21 10:35:57 EDT
Since Hans' Reviews are all done I'm going to review this one. Stay tuned.
Comment 3 Christoph Wickert 2008-06-21 12:03:22 EDT
Fix - MUST: rpmlint: rpmlint /var/lib/mock/fedora-rawhide-i386/result/typespeed-*
typespeed.i386: E: zero-length /var/games/typespeed.score
typespeed.i386: W: file-not-utf8 /usr/share/doc/typespeed-0.6.4/ChangeLog
typespeed.i386: E: setgid-binary /usr/bin/typespeed games 02755
typespeed.i386: E: non-standard-executable-perm /usr/bin/typespeed 02755

The errors are due to the games group ownership and are therefor save to ignore
but please convert the ChangeLog to UTF-8 with iconv. This should be done
together with the sed command in %prep and not during %install

OK - MUST: The package is named according to the  Package Naming Guidelines
OK - MUST: The spec file name matches the base package %{name}
OK - MUST: The package meets the  Packaging Guidelines
OK - MUST: The package is licensed with a Fedora approved license (GPLv2+)
OK - MUST: The License field in the package spec file matches the actual license.
OK - MUST: The source package includes the text of the license and it is
correctly included in %doc
OK - MUST: The spec file is written in American English
OK - MUST: The spec file is legible
OK - MUST: The sources used to build the package match the upstream source by
md5 fb55b92ad7e29a1a6a7a3e1ca383d5e2
OK - MUST: The package successfully compiles and builds into binary rpms on i386
OK - MUST: No known ExcludeArch
OK - MUST: All build dependencies are listed in BuildRequires
OK - MUST: The spec file handles locales properly with the %find_lang macro
OK - MUST: The package is not designed to be relocatable
OK - MUST: The package owns all directories that it creates
OK - MUST: The package does not contain any duplicate files in the %files listing

FIX - MUST: Although permissions on files are set properly, the %defattr is
wrong. Should be "%defattr(-,root,root,-)" for _all_ files, different
permissions should be set with %attr, for example
  %attr(2755,root,games) %{_bindir}/%{name}
  %attr(664,root,games) %{_localstatedir}/games/%{name}.score

OK - MUST: The package has a %clean section, which contains rm -rf %{buildroot}
OK - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines
OK - MUST: The package contains code, no content
OK - MUST: No large documentation files for a separate %{name}-doc package
OK - MUST: The files included as %doc do not affect the runtime of the application

Fix - MUST: The package includes a %{name}.desktop file (although it it no GUI
application) which is correctly installed with desktop-file-install. But the
Category "Application" is obsolete and should be removed. I also suggest to
include typespeed.desktop as a separate file and not to create it in the spec.

OK - MUST: The package does not own files or directories already owned by other
packages
OK - MUST: At the beginning of %install the package runs 'rm -rf %{buildroot}'
OK - MUST: All filenames in the package are valid UTF-8
OK - SHOULD: The package builds in mock
OK - SHOULD: The package functions as described, but I noticed a small bug: The
German words file contains some English words, for example "Equipment".

FIX - Minor: %description should be reworked a little: DOS is an abbreviation
and should therefor be written in capital letters. The third sentence needs an
article at the beginning, "The Idea" instead of "Idea". I suggest you simply use
the description from the 'About' paragraph of the homepage:
"Typespeed's idea is ripped from ztspeed (a DOS game made by Zorlim). The Idea
behind the game is rather easy: type words that are flying by from left to right
as fast as you can. If you miss 10 or more words, game is over.

You can play typespeed for your own or with a friend using TCP/IPv4."
Comment 4 Christoph Wickert 2008-07-11 17:00:11 EDT
Ping?!

ChangeLog and description are no blockers, but the %files section and the
desktop file really should be fixed.
Comment 5 Michel Alexandre Salim 2008-07-14 16:12:25 EDT
Updated spec & SRPM:

Spec URL: http://salimma.fedorapeople.org/for_review/games/typespeed.spec
SRPM URL:
http://salimma.fedorapeople.org/for_review/games/typespeed-0.6.4-2.fc9.src.rpm

Thanks for the review -- sorry for the delay.
Comment 6 Christoph Wickert 2008-07-14 16:54:41 EDT
The package
9ad47f3cc9af37116e7c4c47e54e55e1  typespeed-0.6.4-2.fc9.src.rpm
fixes all blockers:
OK - MUST: %files section fixed
OK - MUST: desktop file is valid
OK - SHOULD: ChangeLog is UTF-8
OK - SHOULD: Description was reworked

Two more suggestions:
- Please remove INSTALL from %doc. It's generic and not needed when installing
from rpm
- Consider adding an icon to the desktop file. I suggest ether "keyboard" or
simply "terminal" since this is a console application.

Anyway: None of these is a blocker, so this package is APPROVED.
Comment 7 Michel Alexandre Salim 2008-07-15 15:47:00 EDT
Keyboard icon sounds perfect -- thanks for the suggestion!

New Package CVS Request
=======================
Package Name: typespeed
Short Description: Test your typing speed and get your fingers' CPS
Owners: salimma
Branches: F-8 F-9 EL-5
InitialCC:
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2008-07-15 18:06:16 EDT
cvs done.
Comment 9 Fedora Update System 2008-07-16 22:01:37 EDT
typespeed-0.6.4-2.fc9 has been submitted as an update for Fedora 9
Comment 10 Fedora Update System 2008-07-16 22:02:21 EDT
typespeed-0.6.4-2.fc8 has been submitted as an update for Fedora 8
Comment 11 Fedora Update System 2008-07-19 05:43:24 EDT
typespeed-0.6.4-2.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update typespeed'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-6538
Comment 12 Fedora Update System 2008-07-31 21:49:04 EDT
typespeed-0.6.4-2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2008-07-31 21:49:21 EDT
typespeed-0.6.4-2.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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