Bug 251950 - Review Request: moodbar - Identifies the "mood" of your music files
Summary: Review Request: moodbar - Identifies the "mood" of your music files
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Debarshi Ray
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords: Reopened
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-08-13 16:29 UTC by Aurelien Bompard
Modified: 2008-08-02 23:40 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-26 19:51:28 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
debarshir: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Aurelien Bompard 2007-08-13 16:29:24 UTC
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/moodbar/moodbar.spec
SRPM URL: http://gauret.free.fr/fichiers/rpms/fedora/moodbar/moodbar-0.1.2-1.fc7.src.rpm
Description: 
Moodbar is a program that takes a music file and generates a .mood file
identifying the "mood" of the music.
It can be used by Amarok to create playlists based on your mood.

Comment 1 Aurelien Bompard 2007-08-13 16:48:26 UTC
Hmm, actually amarok does not use it to create playlists, but to show a visual
representation of what happens in the song. See the home page for more info.
I've updated the description.

Comment 2 Debarshi Ray 2007-12-28 19:21:44 UTC
MUST Items: 

OK - rpmlint is clean on SRPM, RPM and installed package

xx - follows Package Naming Guidelines
   + although the Debian package is named 'moodbar', PLD-Linux, OpenSUSE and
FreeBSD call it 'gstreamer-plugins-moodbar'; in fact the README says Moodbar is
the algorithm and the package contains a GStreamer plugin and an application --
what do you think?

OK - spec file is named as %{name}.spec

xx - package meets Packaging Guidelines
   + consider using %{version} in Source0 to avoid bumping it every time
   + remove the versioned BuildRequires since both F-7 and F-8 have
gstreamer-devel >= 0.10
   + consider using '%configure --disable-static' to avoid building the static
library
   + consider using 'INSTALL="%{__install} -p"' as:
     make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT

OK - license meets Licensing Guidelines
OK - License field meets actual license
OK - upstream license file included in %doc
OK - spec file uses American English
OK - spec file is legible
OK - sources match upstream sources
OK - package builds successfully
OK - ExcludeArch not needed
OK - build dependencies correctly listed
OK - no locales
OK - no shared libraries in any of the dynamic linker's default paths
OK - package is not relocatable
OK - file and directory ownership
OK - no duplicates in %file
OK - file permissions set properly
OK - %clean present
OK - macros used consistently
OK - contains code and permissable content
OK - -doc not needed
OK - contents of %doc does not affect the runtime
OK - no header files
OK - no static libraries
OK - no pkgconfig files
OK - no library files with a suffix
OK - -devel not needed
OK - libtool archives deleted
OK - %{name}.desktop file not needed
OK - does not own files or directories owned by other packages
OK - buildroot correctly prepped
OK - all file names valid UTF-8

SHOULD Items:

OK - upstream provides license text
xx - no translations for description and summary
OK - package builds in mock successfully
OK - package builds on all supported architectures

xx - package functions as expected
   + moodbar crashes -- Debian package has a patch to fix incorrect interaction
with GLib threading system

OK - scriptlets not needed
OK - subpackages not needed
OK - no pkgconfig files
OK - no file dependencies

Here is a patch to fix some of these issues:
http://rishi.fedorapeople.org/moodbar.spec.patch

Comment 3 Aurelien Bompard 2007-12-29 09:34:09 UTC
> xx - follows Package Naming Guidelines
>    + although the Debian package is named 'moodbar', PLD-Linux, OpenSUSE and
> FreeBSD call it 'gstreamer-plugins-moodbar'; in fact the README says Moodbar is
> the algorithm and the package contains a GStreamer plugin and an application --
> what do you think?

Our guideline is "follow the tarball's name", so I'd rather go with "moodbar".

> xx - package meets Packaging Guidelines
>    + consider using %{version} in Source0 to avoid bumping it every time

Okay, why not.

>    + remove the versioned BuildRequires since both F-7 and F-8 have
> gstreamer-devel >= 0.10

I'd rather keep it, in case someone wants to rebuild it on an older
distribution, or in a spinoff of Fedora which would have and older gstreamer.

>    + consider using '%configure --disable-static' to avoid building the static
> library

Done.

>    + consider using 'INSTALL="%{__install} -p"' as:
>      make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT

Done.

> xx - package functions as expected
>    + moodbar crashes -- Debian package has a patch to fix incorrect interaction
> with GLib threading system

Added.


Thanks a lot for you review !

http://gauret.free.fr/fichiers/rpms/fedora/moodbar/moodbar-0.1.2-2.fc8.src.rpm

Comment 4 Debarshi Ray 2007-12-30 05:50:17 UTC
>> xx - follows Package Naming Guidelines
>>    + although the Debian package is named 'moodbar', PLD-Linux, OpenSUSE and
>> FreeBSD call it 'gstreamer-plugins-moodbar'; in fact the README says Moodbar is
>> the algorithm and the package contains a GStreamer plugin and an application --
>> what do you think?
 
> Our guideline is "follow the tarball's name", so I'd rather go with "moodbar".

Umm... We have gstreamer-plugins-farsight, farsight and farsight-devel in
Fedora. However they are distributed as separate plugins. Would it make sense to
split this into moodbar and gstreamer-plugins-moodbar, or you consider it as an
overkill?

>>    + remove the versioned BuildRequires since both F-7 and F-8 have
>> gstreamer-devel >= 0.10
 
> I'd rather keep it, in case someone wants to rebuild it on an older
> distribution, or in a spinoff of Fedora which would have and older gstreamer.

http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#Requires
says: "As a rule of thumb, if the version is not required, don't add it just for
fun." But that might be a suggestion and not a requirement. I will leave this to
your judgement.


Comment 5 Aurelien Bompard 2007-12-30 11:11:41 UTC
> Umm... We have gstreamer-plugins-farsight, farsight and farsight-devel in
> Fedora. However they are distributed as separate plugins. Would it make sense to
> split this into moodbar and gstreamer-plugins-moodbar, or you consider it as an
> overkill?

I'd say it's overkill for such a small plugin. I'd rather KISS when possible.

http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#Requires
> says: "As a rule of thumb, if the version is not required, don't add it just for
> fun." But that might be a suggestion and not a requirement. I will leave this to
> your judgement.

Yeah, that makes sense too. I've removed it.
 
http://gauret.free.fr/fichiers/rpms/fedora/moodbar/moodbar-0.1.2-3.fc8.src.rpm


Comment 6 Debarshi Ray 2007-12-30 13:05:44 UTC
+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+

Comment 7 Aurelien Bompard 2007-12-30 14:25:16 UTC
New Package CVS Request
=======================
Package Name: moodbar
Short Description: Identifies the mood of your music files
Owners: abompard
Branches: F-7 F-8
Cvsextras Commits: yes

Comment 8 Kevin Fenzi 2007-12-30 17:52:11 UTC
cvs done.

Comment 9 Fedora Update System 2008-01-07 01:14:42 UTC
moodbar-0.1.2-3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2008-01-07 01:19:43 UTC
moodbar-0.1.2-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 11 Debarshi Ray 2008-01-07 13:26:55 UTC
I think this should be now closed as "NEXTRELEASE".

Comment 12 Debarshi Ray 2008-01-25 17:57:34 UTC
Is there any outstanding issue regarding this? If not, then this should be
closed as "NEXTRELEASE".

Comment 13 Aurelien Bompard 2008-01-26 19:51:28 UTC
Thanks


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