Bug 251950 - Review Request: moodbar - Identifies the "mood" of your music files
Review Request: moodbar - Identifies the "mood" of your music files
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Debarshi Ray
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-13 12:29 EDT by Aurelien Bompard
Modified: 2008-08-02 19:40 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-26 14:51:28 EST
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)

  None (edit)
Description Aurelien Bompard 2007-08-13 12:29:24 EDT
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 12:48:26 EDT
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 14:21:44 EST
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 04:34:09 EST
> 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 00:50:17 EST
>> 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 06:11:41 EST
> 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 08:05:44 EST
+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+
Comment 7 Aurelien Bompard 2007-12-30 09:25:16 EST
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 12:52:11 EST
cvs done.
Comment 9 Fedora Update System 2008-01-06 20:14:42 EST
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-06 20:19:43 EST
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 08:26:55 EST
I think this should be now closed as "NEXTRELEASE".
Comment 12 Debarshi Ray 2008-01-25 12:57:34 EST
Is there any outstanding issue regarding this? If not, then this should be
closed as "NEXTRELEASE".
Comment 13 Aurelien Bompard 2008-01-26 14:51:28 EST
Thanks

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