Bug 190397 - Review Request: netpanzer-data - Data files for netpanzer
Review Request: netpanzer-data - Data files for netpanzer
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 190396
  Show dependency treegraph
 
Reported: 2006-05-01 17:58 EDT by Hugo Cisneiros
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-06 11:31:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hugo Cisneiros 2006-05-01 17:58:07 EDT
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-01 21:46:54 EDT
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-01 22:57:24 EDT
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-01 23:06:46 EDT
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 10:56:36 EDT
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 17:02:04 EDT
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 17:09:06 EDT
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 17:11:58 EDT
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 17:45:23 EDT
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 03:57:10 EDT
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@redhat.com>
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 14:32:51 EDT
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 11:31:54 EDT
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 08:51:20 EST
Change owner to limb@jcomserv.net (orphaned)
Comment 13 Gwyn Ciesla 2007-03-01 14:42:04 EST
Add lxtnow@gmail.com as co-maintainer.

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