Bug 200374 - Review Request: qstat - Real-time Game Server Status for Quake servers
Review Request: qstat - Real-time Game Server Status for Quake servers
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-07-27 04:55 EDT by Andy Shevchenko
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-01 10:49:57 EDT
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 Andy Shevchenko 2006-07-27 04:55:20 EDT
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 17:08:11 EDT
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 09:14:48 EDT
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@asplinux.com.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 15:47:40 EDT
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 04:22:42 EDT
Updated package here:
ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat-2.10-4.src.rpm
Comment 5 Wart 2006-07-31 12:37:12 EDT
All MUSTFIX items fixed.

APPROVED
Comment 6 Andy Shevchenko 2006-08-01 10:49:57 EDT
Injected into CVS.

Thanks for review.

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