Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 200374 - Review Request: qstat - Real-time Game Server Status for Quake servers
Summary: Review Request: qstat - Real-time Game Server Status for Quake servers
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Wart
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-07-27 08:55 UTC by Andy Shevchenko
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-01 14:49:57 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Andy Shevchenko 2006-07-27 08:55:20 UTC
Spec URL: ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat.spec
SRPM URL: ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat-2.10-2.src.rpm
Description:
QStat is a command-line program that gathers real-time statistics
from Internet game servers. Most supported games are of the first
person shooter variety (Quake, Half-Life, etc)

Comment 1 Wart 2006-07-27 21:08:11 UTC
A few small points before I get to a full review:

You don't need the check for "/" when you clean the buildroot in %install and
%clean.  A simple rm -rf will suffice:
rm -rf %{buildroot}

The -n %{name}-%{version} is unnecessary.  %setup already uses this as a default.

The %attr statements for qstat.cfg and the qstat binary are also not necessary
as these permissions/ownership are already used.

qstat can be used for more than just quake servers, so remove 'quake' from the
summary line.  You could replace it with 'FPS game' instead.

Comment 2 Andy Shevchenko 2006-07-28 13:14:48 UTC
Updated package here:
ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat-2.10-3.src.rpm

* Fri Jul 28 2006 Andy Shevchenko <andriy.ua> 2.10-3
- drop check for "/" in install and clean sections
- drop -n for setup macro
- do not use attr macro in files section
- qstat can be used not only for Quake, so change Summary


Comment 3 Wart 2006-07-29 19:47:40 UTC
GOOD
====
* rpmlint output clean
* Package and spec file name named appropriately
* Artistic license ok, license file included
* File matches upstream:
  ac3ce3dbed5248bd5738a4968460880e  qstat-2.10.tar.gz
* Spec file legible and in Am. English
* Builds and packages in mock on FC4, FC5, and FC5, both i386 and x86_64
* Package provides list is sane
* No BR: necessary
* No locales
* No shared libs
* Not relocatable
* Does not create any directories that it should own
* No duplicate %files
* File permissions ok
* build root cleaned in %install and %clean as necessary
* Contains code, not content
* No need for -doc or -devel subpackages
* No .la files created
* Not a gui app; no .desktop file needed

MUSTFIX
=======
* %doc contains some unnecessary files that should be removed:
  COMPILE.txt
  Makefile*
  template/Makefile*

NOTES
=====
* There are a number of compiler warnings about pointer signedness that appear
harmless.  If you feel inclined, you could report these upstream.
qstat.c:2912: warning: pointer targets in passing argument 6 of 'recvfrom'
differ in signedness


Comment 4 Andy Shevchenko 2006-07-31 08:22:42 UTC
Updated package here:
ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat-2.10-4.src.rpm


Comment 5 Wart 2006-07-31 16:37:12 UTC
All MUSTFIX items fixed.

APPROVED

Comment 6 Andy Shevchenko 2006-08-01 14:49:57 UTC
Injected into CVS.

Thanks for review.


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