Bug 519986 - media-player-info - Data files describing media player capabilities
Summary: media-player-info - Data files describing media player capabilities
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Bastien Nocera
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-27 21:22 UTC by Matthias Clasen
Modified: 2009-09-02 03:46 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-09-02 03:46:20 UTC
Type: ---
Embargoed:
bnocera: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Matthias Clasen 2009-08-27 21:22:01 UTC
spec: http://people.redhat.com/mclasen/media-player-id.spec
srpm: http://people.redhat.com/mclasen/media-player-id-1-1.fc12.src.rpm
description: 

media-player-id is a repository of data files describing media player
(mostly USB Mass Storage ones) capabilities. These files contain information
about the directory layout to use to add music to these devices, about the
supported file formats, etc.

The package also installs a udev rule to identify media player devices.

Comment 1 Bill Nottingham 2009-08-28 19:06:28 UTC
MUST items:
- Package meets naming and packaging guidelines - OK
- Spec file matches base package name. - OK
- Spec has consistant macro usage. - OK
- Meets Packaging Guidelines. - OK
- License - ***

1) This package consists of nothing but configuration files and a udev rules file. These are configuration, and not necessarily copyrightable.
2) Spot says "use a permissive license just to be sure."
3) Which implies the BSD in the tarball is OK...
4) except it still states:

Copyright (c) The Regents of the University of California.

which is almost certainly wrong, in the case that the package *is* copyrightable. Should be Christophe & Martin, or whomever.

- License field in spec matches - OK, as it stands
- License file included in package - OK
- Spec in American English - OK
- Spec is legible. - OK
- Sources match upstream md5sum:

878fd1b6a8baccf0ce46d29f2fde559a40d6573c  media-player-id-1.tar.gz

OK.

- Package needs ExcludeArch - ***

Might want 'ExcludeArch: s390 s390x <other similar things>'. But I doubt it matters that much.

- BuildRequires correct - OK. 
- Spec handles locales/find_lang - N/A
- Package has %defattr and permissions on files is good. - OK
- Package has a correct %clean section. - OK
- Package has correct buildroot - OK
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
- Package is code or permissible content. - OK
- Doc subpackage needed/used. - N/A
- Packages %doc files don't affect runtime. - OK

- Package compiles and builds on at least one arch. - OK (tested F12)
- Package has no duplicate files in %files. - OK
- Package doesn't own any directories other packages own. - OK
- Package owns all the directories it creates. - OK
- No rpmlint output. - OK
- final provides and requires are sane: - OK

Can we get them to fix the licensing?

Comment 2 Matthias Clasen 2009-08-28 20:30:19 UTC
> Might want 'ExcludeArch: s390 s390x <other similar things>'

Why ? Is there a reason, other than 'media players are pretty irrelevant on s390' ?

> Can we get them to fix the licensing?  

I'll point that out to them. I might hold off a few days on completing this review anyway, since Christophe was considering renaming it to media-player-info.

Comment 3 Bill Nottingham 2009-08-28 20:32:04 UTC
(In reply to comment #2)
> > Might want 'ExcludeArch: s390 s390x <other similar things>'
> 
> Why ? Is there a reason, other than 'media players are pretty irrelevant on
> s390' ?

No, not really.

Comment 4 Peter Lemenkov 2009-08-29 08:19:32 UTC
(In reply to comment #1)
> - Package needs ExcludeArch - ***
> 
> Might want 'ExcludeArch: s390 s390x <other similar things>'. But I doubt it
> matters that much.

I don't think that Matthias should add this line. If he adds it, then he would create bugzilla tickets (for each excluded arch), describing reasons why this package is excluded from articular architectures, but we haven't such reasons except "I'm not sure that someone will use it there".

Actually, I also can't imagine, that someone will plug his iPod to s390x mainframe, but there are no technical issues preventing us from building this package on every available arch.

Comment 5 Matthias Clasen 2009-08-29 16:12:39 UTC
Renamed packages:

spec: http://people.redhat.com/mclasen/media-player-info.spec
srpm: http://people.redhat.com/mclasen/media-player-info-2-1.fc12.src.rpm

I've mentioned the Copyright headers to Christophe, but that fix is not included yet.

Comment 7 Bastien Nocera 2009-09-01 16:07:35 UTC
Picking up from Bill.

The copyright issue was fixed, and rpmlint is clean, so this looks good.

Comment 8 Matthias Clasen 2009-09-01 16:11:46 UTC
New Package CVS Request
=======================
Package Name: media-player-info
Short Description: Data files describing media player capabilities
Owners: mclasen
Branches: 
InitialCC:

Comment 9 Matthias Clasen 2009-09-02 03:46:20 UTC
build done.


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