Bug 200374
| Summary: | Review Request: qstat - Real-time Game Server Status for Quake servers | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Andy Shevchenko <andy> |
| Component: | Package Review | Assignee: | Wart <wart> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | wart |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2006-08-01 14:49:57 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 163779 | ||
|
Description
Andy Shevchenko
2006-07-27 08:55:20 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.
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 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 Updated package here: ftp://andriy.asplinux.com.ua/pub/people/andy/extras/qstat-2.10-4.src.rpm All MUSTFIX items fixed. APPROVED Injected into CVS. Thanks for review. |