Bug 527840 - Review Request: parole - Media player for the Xfce desktop
Summary: Review Request: parole - Media player for the Xfce desktop
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-07 21:39 UTC by Christoph Wickert
Modified: 2009-11-04 12:15 UTC (History)
3 users (show)

Fixed In Version: 0.1.91-1.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-13 04:14:59 UTC
Type: ---
Embargoed:
kevin: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Christoph Wickert 2009-10-07 21:39:32 UTC
Spec URL: http://cwickert.fedorapeople.org/review/parole.spec
SRPM URL: http://cwickert.fedorapeople.org/review/parole-0.1.90-1.fc12.src.rpm
Description: Parole is a modern simple media player based on the GStreamer framework and written to fit well in the Xfce desktop. Parole features playback of local media files, DVD/CD and live streams. Parole is extensible via plugins.

The project still in its early developments stage, but already contains the following features:
* Audio playback
* Video playback with optional subtitle
* Playback of live sources


Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1734083

Notes: The 'Mute' menu entry doesn't work. Should I remove it from the menu for now?

Comment 1 Kevin Fenzi 2009-10-07 21:41:40 UTC
I'd be happy to review this. Look for a full review later tonight.

Comment 2 Kevin Fenzi 2009-10-08 05:17:10 UTC
OK - 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 GPLv2+
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
acf9085b49cf41469ce41076e6915253  parole-0.1.90.tar.bz2
acf9085b49cf41469ce41076e6915253  parole-0.1.90.tar.bz2.orig
See below - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage. 
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch. 
OK - 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 - Package obey's FHS standard (except for 2 exceptions)
OK - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have sane scriptlets. 
OK - Should have subpackages require base package with fully versioned depend. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Notes:

Two of the files are LGPLv2+, but that should make the package still GPLv2+. 
(parole-subtitle-encoding.[ch])

rpmlint is silent. 

Issues: 

1. A few optional packages we might be able to build with: 

checking for optional package libnotify >= 0.4.1... not found
checking for optional package taglib >= 1.4... not found

Other than that I don't see any blockers, so this package is APPROVED. 
Check into the optional packages above before importing. 

Let me know if you would like a co-maintainer, happy to help.

Comment 3 Christoph Wickert 2009-10-08 17:12:03 UTC
(In reply to comment #2)

> OK - Package obey's FHS standard (except for 2 exceptions)

What do you mean with that? If you are referring to the unversioned libs in %{_libdir}/%{name}-0, which according to our guidelines should be in the -devel package: these are the plugins. They are unversioned as they are not used by anything other than parole.

> Two of the files are LGPLv2+, but that should make the package still GPLv2+. 
> (parole-subtitle-encoding.[ch])

Right, the whole app itself still is GPLv2+

> 1. A few optional packages we might be able to build with: 
> 
> checking for optional package libnotify >= 0.4.1... not found
> checking for optional package taglib >= 1.4... not found

Oops, obviously I didn't look close enough at the build logs. One was a typo (libnotify instead of libnotify-devel), the other was simply missing (taglib-devel >= 1.4). I have fixed both in the spec:
http://cwickert.fedorapeople.org/review/parole.spec

> Let me know if you would like a co-maintainer, happy to help.  

Great, that would be helpful. I'll add you as owner.


New Package CVS Request
=======================
Package Name: parole
Short Description: Media player for the Xfce desktop
Owners: cwickert kevin
Branches: F-11 F-12
InitialCC:

Comment 4 Kevin Fenzi 2009-10-08 20:35:05 UTC
>> OK - Package obey's FHS standard (except for 2 exceptions)

>What do you mean with that? If you are referring to the unversioned libs in
>%{_libdir}/%{name}-0, which according to our guidelines should be in the -devel
>package: these are the plugins. They are unversioned as they are not used by
>anything other than parole.

Nothing to do with this package, thats part of my regular checklist. 
The guidelines say packages should follow FHS, but list 2 exceptions where that should not be the case. :) 
"There are notable exceptions to this guideline for libexecdir (as specified in the [Coding Standards]) and /usr/target for cross-compilers."

The new spec looks good, happy to co-maintain. Thanks.

Comment 5 Kevin Fenzi 2009-10-09 02:54:53 UTC
cvs done.

Comment 6 Fedora Update System 2009-10-09 09:14:34 UTC
parole-0.1.90-2.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/parole-0.1.90-2.fc12

Comment 7 Fedora Update System 2009-10-09 09:15:22 UTC
parole-0.1.90-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/parole-0.1.90-2.fc11

Comment 8 Kevin Fenzi 2009-10-13 04:14:59 UTC
Imported and built. Closing now.

Comment 9 Fedora Update System 2009-11-04 12:15:23 UTC
parole-0.1.91-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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