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 190397 - Review Request: netpanzer-data - Data files for netpanzer
Summary: Review Request: netpanzer-data - Data files for netpanzer
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 190396
TreeView+ depends on / blocked
 
Reported: 2006-05-01 21:58 UTC by Hugo Cisneiros
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-05-06 15:31:54 UTC
Type: ---
Embargoed:
petersen: fedora-cvs+


Attachments (Terms of Use)

Description Hugo Cisneiros 2006-05-01 21:58:07 UTC
Spec URL: http://www.devin.com.br/eitch/rpm/fedora/SPECS/netpanzer-data.spec
SRPM URL: http://www.devin.com.br/eitch/rpm/fedora/5/SRPMS/netpanzer-data-0.8-1.src.rpm
Description: 

This package contains the graphics and sounds for netpanzer, an online
multiplayer tactical warfare game designed for FAST ACTION combat.

This is my first set of packages, so I need a sponsor please :)

Comment 1 Andreas Thienemann 2006-05-02 01:46:54 UTC
Good:
 * proper naming
 * spec file name matches %{name}
 * package meets packaging guidelines
 * License is GPL, License meets packaged COPYING
 * Spec file written in American English
 * Spec file is understandable
 * Package succesfully builds in mock on devel x86_64 and FC-5 x86
 * No locales/shared libraries to worry about
 * No static/libtool files
 * Package not relocatable
 * Package owns all directories it creates
 * No duplicate files
 * Proper file permissions, proper %defattr(...) in spec file
 * Package contains necessary game-data for netpanzer, conforming to the
Packaging Guidelines
 * No need for separate doc package
 * %doc files not needed for runtime
 * No header/other devel package files to worry about
 * Desktop File not needed
 * Package is correctly set to noarch.

Approved, pending sponsorship and successfull review of #190396

Comment 2 Hugo Cisneiros 2006-05-02 02:57:24 UTC
Updated package:

Spec URL: http://www.devin.com.br/eitch/rpm/fedora/SPECS/netpanzer-data.spec
SRPM URL:
http://www.devin.com.br/eitch/rpm/fedora/5/SRPMS/netpanzer-data-0.8-2.src.rpm

Changes: 

- Changed Package's RPM Group
- Fixed Changelog entries to specify versions
- Stripped '\r' EOL from RELNOTES file

Comment 3 Andreas Thienemann 2006-05-02 03:06:46 UTC
Thanks for updating the package with my suggestions as well as the other
suggestions on IRC.
Gonna take a look at the package tomorrow though.

Comment 4 Hans de Goede 2006-05-04 14:56:36 UTC
I can sponsor and you seem worthy of sponsering concedering your quick and
correct reactions to this review and your other opensource and Fedora
(translation / writing) work.

Assigning to me. I'll do a Review as time permits.





Comment 5 Hans de Goede 2006-05-04 21:02:04 UTC
I'll keep my review short (again) as Andreas has already done most of the work.
I only see 1 problem, why all the BuildReqs? I see no need for
desktop-file-utils and most of the others seem superficial too.


Comment 6 Hugo Cisneiros 2006-05-04 21:09:06 UTC
While contents of this packages includes only data-files, they are included with
a ./configure script and jam (make substitute). This is really strange indeed,
but this configure searchs for these dependencies, just like the main netpanzer
package (Bug #190396). As I like to work in the ways upstream works, I thought
that it would be good to follow this scheme.

But you're right about desktop-file-utils, it was a copy and paste error of
mine, sorry :) I'm updating the spec and rebuilding the package after you
comment about my opinion above. Thanks!

Comment 7 Hans de Goede 2006-05-04 21:11:58 UTC
I already thought it might be such a thing (a strange configure script) in that
case leaving the BR's in is fine. A comment in the .spec about this might be a
good idea though.


Comment 8 Hugo Cisneiros 2006-05-04 21:45:23 UTC
Updated package:

Spec URL: http://www.devin.com.br/eitch/rpm/fedora/SPECS/netpanzer-data.spec
SRPM URL:
http://www.devin.com.br/eitch/rpm/fedora/5/SRPMS/netpanzer-data-0.8-3.src.rpm

Changes: 

- Removed desktop-file-utils BuildReq entry

Notes:

Commenting the BuildReq indeed is good, so I did it ;)

Comment 9 Hans de Goede 2006-05-05 07:57:10 UTC
Looking good, _approved_!

One request though, could you remove the %{?dist} from the release field before
importing (don't forget to rebuild the .src.rpm)?

There has been some discussion about large .noarch content packages like this
eating up a lot of diskspace on the mirrors. So the idea is to build it once and
then copy it over to the other branches. So you also won't need to request an
FC-5 branch for this on the CVS-admin page. Instead once its build for devel
(GRRR building on devel i still broken), mail Warren Togami <wtogami>
with a request to copy it to the FC-5 repo. This was disccused here:
https://www.redhat.com/archives/fedora-games-list/2006-May/msg00001.html

Currently this doesn't help the mirrors, but in the future the copy may be
replaced by a link.


Comment 10 Hugo Cisneiros 2006-05-05 18:32:51 UTC
Updated package:

Spec URL: http://www.devin.com.br/eitch/rpm/fedora/SPECS/netpanzer-data.spec
SRPM URL:
http://www.devin.com.br/eitch/rpm/fedora/5/SRPMS/netpanzer-data-0.8-3.src.rpm

Notes:

Like Comment #9 said, removed the %{?dist} var from the Release field.

Comment 11 Hugo Cisneiros 2006-05-06 15:31:54 UTC
Package built for devel, and will be copied by Warren Togami for the other
branches. I think the work is completed, closing ;)

Comment 12 Gwyn Ciesla 2007-02-28 13:51:20 UTC
Change owner to limb (orphaned)

Comment 13 Gwyn Ciesla 2007-03-01 19:42:04 UTC
Add lxtnow as co-maintainer.


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