Bug 431335 - Review Request: decibel-audio-player - Music player for GNOME
Review Request: decibel-audio-player - Music player for GNOME
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: John Mahowald
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2008-02-02 18:59 EST by Debarshi Ray
Modified: 2008-05-25 14:59 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-25 14:59:32 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jpmahowald: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Debarshi Ray 2008-02-02 18:59:32 EST
Spec URL: http://rishi.fedorapeople.org/decibel-audio-player.spec
SRPM URL: http://rishi.fedorapeople.org/decibel-audio-player-0.09-1.fc8.src.rpm

Description:

Decibel is an audio player that aims at being very straightforward to use by
means of a very clean and user friendly interface. It is especially targeted
at GNOME and will follow, as closely as possible, the GNOME HIG. It makes use
of the GStreamer library to read audio files.
Comment 1 John Mahowald 2008-02-13 15:36:02 EST
Does build and play on development.

However, it must install to %{python_sitelib}  as per
http://fedoraproject.org/wiki/Packaging/Python  
At the moment it's /usr/share/decibel-audio-player

Speaking of decibel-audio-player, why patch the .desktop to be Decibel Music
Player? Every other reference, from the package name to the about box is Decibel
Audio Player. I request it be Audio Player in the .desktop.

Why not use the default BuildRoot of 
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) ?


Good:
Source matches
License of GPLv2+  matches source headers
Comment 2 Debarshi Ray 2008-02-19 12:38:06 EST
(In reply to comment #1)

> However, it must install to %{python_sitelib}  as per
> http://fedoraproject.org/wiki/Packaging/Python  
> At the moment it's /usr/share/decibel-audio-player
 
Those Python files are only meant to be used by Decibel, and not any other
program. Only system-wide files (or modules) should go in %{python_sitelib},
isn't it?

> Speaking of decibel-audio-player, why patch the .desktop to be Decibel Music
> Player?

To ensure consistency in the desktop menus. We have "Banshee Music Player",
"Exaile Music Player", etc.. So the trend seems to be "<name> Music Player" for
such applications.

What do you suggest?

> Why not use the default BuildRoot of 
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) ?

That used to be the default BuildRoot. Not anymore. See:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473
Comment 3 Debarshi Ray 2008-04-24 22:09:11 EDT
This review is stalled. If there is no response from John Mahowald within a
week, I will remove the FE-REVIEW blocker and replace it with FE-NEW.

http://fedoraproject.org/wiki/PackageMaintainers/Policy/StalledReviews
Comment 4 John Mahowald 2008-05-20 23:08:23 EDT
Good point regarding python files. They are arch independent so leave them in
%{_datadir}

I'll leave it to you to decide Music or Audio player, Music is fine. Both work
fine in English. The key word users will see is Decibel anyways.

Nice citation of the guidelines, that BuildRoot does meet the requirements.


So:

+ Still builds in development
+ Works
+ rpmlint is clean
+ Includes byte compiled files
+ License matches, GPLv2+
+ Proper Requires and BuildRequires
+ Installed .desktop file, appears under "Sound and Video"
+ Proper use of macros


APPROVED
Comment 5 Debarshi Ray 2008-05-21 00:32:03 EDT
New Package CVS Request
=======================
Package Name: decibel-audio-player
Short Description: Music player for GNOME
Owners: rishi
Branches: F-7 F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 6 Kevin Fenzi 2008-05-21 14:29:21 EDT
cvs done.

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