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 227873 (sear-media) - Review Request: sear-media - media files for the sear game client
Summary: Review Request: sear-media - media files for the sear game client
Keywords:
Status: CLOSED NEXTRELEASE
Alias: sear-media
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: sear
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-02-08 18:21 UTC by Wart
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: 2007-02-22 03:25:43 UTC
Type: ---
Embargoed:
chris.stone: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Wart 2007-02-08 18:21:12 UTC
Spec URL: http://www.kobold.org/~wart/fedora/sear-media.spec
SRPM URL: http://www.kobold.org/~wart/fedora/sear-media-0.6-1.src.rpm
Description: 

Media files for the sear WorldForge client.

Note to reviewer:  The dist tag is intentionally not used as this is a large noarch blob of game data that doesn't need to be rebuilt, only copied, between fedora releases.

Comment 1 Christopher Stone 2007-02-20 17:43:39 UTC
==== REVIEW CHECKLIST ====
- rpmlint output
W: sear-media hidden-file-or-dir
/usr/share/sear/sear-media-0.6/castle/.dot_it.sh.swp

Looks like this file can safely be removed.
- package named according to package naming guidelines
- spec file name matches %{name}
- package meets packaging guidelines
- licensed with open source compatible license
X license tag matches actual license
- license file included in %doc
- spec written in American english
- spec file legible
- sources match upstream
c136577e5ca64dd39a91d47c0c4c2ba6  sear-media-20070206.tar.gz
- package successfully compiles and builds on FC-6 x86_64
- all build dependencies listed in BR
- no locales
- no shared libraries
- package is not relocatable
X package does not own all directories it creates
- no duplicates in %files
- file permissions set properly
- package contains proper %clean
- macro usage consistent
- package contains permissible content
- no large documentation
- files in %doc do not affect runtime
- no header files or static libraries
- no pkgconfig files
- no library files with suffix
- no need for devel subpackage
- no libtool archives
- not a GUI application
- does not own files or directories owned by other packages


==== MUST FIX ====
- Remove .swp file found by rpmlint
- LICENSING.txt is confusing.  First it says the artwork is GPL, but then goes
on to say that the actual license is modifable under the GFDL, and then they
list sections of documentation which are clearly broilerplate sections in an
unmodified license file, for example "Sections being LIST THEIR TITLES", and
"modify this document" when the document itself is a license file.  I guess this
needs to be clarified with upstream? It seems they do not really care.  I guess
I also have to ask why you license this as just GPL instead of GPL/GFDL.
- the package creates a directory "sear" under /usr/share which it does not own,
nor does it pull in any packages which own this directory in Requries.
- README and COPYING.txt probably dont need to be included twice in the file
list, LICENSING.txt explicitly mentions "files under this directory" so I guess
this has to be in both locations, however the license *is* modifyable under the
GFDL... ;-)


Comment 2 Wart 2007-02-20 22:16:21 UTC
Updated package:

http://www.kobold.org/~wart/fedora/sear.spec
http://www.kobold.org/~wart/fedora/sear-media-0.6-2.src.rpm

- editor swap file removed
- I've asked upstream to clarify/cleanup the licensing text, and updated the
spec file to use "License: GPL/GFDL"
- Removed duplicate license texts from %doc and %{_datadir}

I'm not sure of the best way to handle the unowned /usr/share/sear directory. 
This is provided by the 'sear' package, which will 'Requires: sear-media' once
approved.  But this would still allow someone to install sear-media without
sear, and have an unowned directory.  I see two ways to handle this:

1) Have both sear and sear-media own %{_datadir}/sear
2) Have both packages require each other:
   sear 'Requires: sear-media'
   sear-media 'Requires: sear'
...but I thought that circular dependencies were frowned upon.

Comment 3 Christopher Stone 2007-02-21 00:47:13 UTC
Why won't this work?:

sear-media.spec:
...
%files
%dir %{_datadir}/sear
{%_datadir}/sear/%{name}-%{version}

Then in sear.spec:
Requires: %{name}-media
...
%files
%{_datadir}/%{name}/*


Comment 4 Wart 2007-02-21 01:02:07 UTC
Well aren't you just the clever one.  :)
That should work just fine.  According to a small sampling on f-d-l, a circular
dependency in this case would be ok too.

http://www.kobold.org/~wart/fedora/sear-media.spec
http://www.kobold.org/~wart/fedora/sear-media-0.6-3.src.rpm

Comment 5 Christopher Stone 2007-02-21 01:24:17 UTC
All must items fixed.  Approved pending changes to sear.spec mentioned above.


Comment 6 Wart 2007-02-21 01:42:27 UTC
I've made the changes to sear.spec in CVS and will rebuild once sear-media has
been checked in and built.  Thanks for the review!

Comment 7 Wart 2007-02-21 01:45:03 UTC
Package Name: sear-media
Short Description: Media files for the sear WorldForge client
Owners: wart
Branches: FC-6
InitialCC: che666

Comment 8 Wart 2007-02-22 03:25:43 UTC
Imported and built for rawhide.  Thanks!


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