Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 233598 - Review Request: elisa - Media Center
Summary: Review Request: elisa - Media Center
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Xavier Lamien
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 228298 228299 228301 228303 233596 233597
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-23 12:34 UTC by Matthias Saou
Modified: 2009-10-25 19:40 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 530021 (view as bug list)
Environment:
Last Closed: 2007-05-22 08:34:57 UTC
lxtnow: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Matthias Saou 2007-03-23 12:34:21 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.4.2-1.src.rpm
Description:
Media center solution using the GStreamer multimedia framework.

Comment 1 Matthias Saou 2007-03-23 12:37:10 UTC
Note that this package depends on a few python packages as well as the pigment
toolkit, which will need to be reviewed first (see the bug dependencies).

Comment 2 Xavier Lamien 2007-03-23 16:11:27 UTC
Okay, 

i'will review all request dependencies before start this one

Comment 3 Matthias Saou 2007-04-16 11:01:33 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.5-1.src.rpm

* Mon Apr 16 2007 Matthias Saou <http://freshrpms.net/> 0.1.5-1
- Update to 0.1.5.
- Disable gst requirements which aren't part of Fedora (oops!).
- Patch out the hash-bang python from scripts not meant to be executed.
- Rip out the root user test condition to installing the desktop entry.

Comment 4 Jef Spaleta 2007-05-04 06:40:53 UTC
Quick pass over the spec file...

You'll need to add the appropriate desktop-file-install call in the install
section and add BuildRequires:  desktop-file-utils

-jef

Comment 5 Jef Spaleta 2007-05-04 06:48:51 UTC
Oh and the URL and SOURCE tags need to be updated, same thing as we went through
with pigment. 

www.fluendo.com -> elisa.fluendo.com  

Looks like elisa 0.1.6 is out according to the website. Do you want to bump up
to 0.1.6 before i start the real review?
http://elisa.fluendo.com/static/download/elisa/elisa-0.1.6.tar.gz

-jef

Comment 6 Jef Spaleta 2007-05-04 06:58:23 UTC
Does python-setuptools need to be a Requires and a BuildRequires?  The Requires
seems odd to me.

-jef

Comment 7 Matthias Saou 2007-05-04 13:02:31 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.6-1.src.rpm

Updated to 0.1.6 and changes the URLs.
IIRC the runtime dependency on python-setuptools is needed, otherwise elisa will
fail miserably (try it if you're unsure ;-))
As for the desktop file installation, I thought "processing" it was now only
required if it wasn't a sane file to begin with (i.e. missing categories, lines,
wrong encoding etc.)... did I make this up? :-/

Oh, and don't forget that coherence (bug #233596) still needs to be reviewed and
imported before elisa.

Comment 8 Matthias Saou 2007-05-07 11:23:07 UTC
Please note that the two patches I include have now been included upstream for
the next Elisa release (the desktop files one in a better form, since a check
still makes sense).

Comment 9 Matthias Saou 2007-05-07 14:42:03 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.6-2.src.rpm

* Mon May  7 2007 Matthias Saou <http://freshrpms.net/> 0.1.6-2
- Change coherence requirement to Coherence to match package name change.

Comment 10 Xavier Lamien 2007-05-07 17:06:28 UTC
Redundant Require:

python-twisted-core is required by Coherence package which is required by elisa.

Comment 11 Matthias Saou 2007-05-07 17:21:17 UTC
(In reply to comment #10)
> Redundant Require:
> 
> python-twisted-core is required by Coherence package which is required by elisa.

I wouldn't consider this a redundant require, since Coherence requires twisted,
but so does Elisa directly. If some day Coherence no longer requires it, or if
Elisa no longer requires Coherence, I want the requirement from within Elisa to
still be there.

Comment 12 Xavier Lamien 2007-05-07 18:48:10 UTC
okay,

The package python-tag (bug #228303) need to be accepted and imported first
before approved this one.
as it's a Require of elisa.



Comment 13 Matthias Saou 2007-05-09 11:18:53 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.6-3.src.rpm

* Tue May  8 2007 Matthias Saou <http://freshrpms.net/> 0.1.6-3
- Change Coherence requirement to python-Coherence to match package name change.

- python-Coherence should be imported and built shortly.
- python-tag is the only dependency left... I didn't hear back from the author
  yet :-(

Comment 14 Matthias Saou 2007-05-14 16:32:22 UTC
All dependencies are now in Fedora (python-tag has just been rebuilt).

It's definitely time for a complete review now... feel free! :-)

(Note that I also have another "Media Center" package waiting for a review :
oxine, based on the xine lib, bug #230549)

Comment 15 Xavier Lamien 2007-05-14 16:45:54 UTC
Well, good to know.

> (Note that I also have another "Media Center" package waiting for a review :
> oxine, based on the xine lib, bug #230549)

I'll check it out.

Starting review...

Comment 16 Xavier Lamien 2007-05-15 22:21:21 UTC
--------------------
From %install stage.
-------------------

you forgot to install the desktop file by using desktop-file-install command and
add desktop-file-utils as BR.

-------------------
From desktop file.
-------------------

you SHOULD Remove Application, X-Ximian-Main and X-Red-Hat-Base from Categories
entry

Comment 17 Matthias Saou 2007-05-16 18:07:35 UTC
> you forgot to install the desktop file by using desktop-file-install
> command and add desktop-file-utils as BR.

I'd prefer avoiding this. If the desktop file is sane enough and already
provided, it's fine to install it directly.

> you SHOULD Remove Application, X-Ximian-Main and X-Red-Hat-Base from
> Categories entry

Sure, I'll patch all this out. One interesting thing is that the current
rhythmbox, gnome-cd and gnome-volume-control desktop files on my FC6 system
contain the exact same lines.

Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/elisa/elisa-0.1.6-4.src.rpm

* Wed May 16 2007 Matthias Saou <http://freshrpms.net/> 0.1.6-4
- Patch desktop file to remove useless bits (Version and extra Categories).

Comment 18 Xavier Lamien 2007-05-17 07:22:04 UTC
>> I'd prefer avoiding this. If the desktop file is sane enough and already
>> provided, it's fine to install it directly.

By using this command you make the desktop file available on the GNOME/KDE/SFCE
menu (and/or panel).



Comment 19 Matthias Saou 2007-05-17 07:41:15 UTC
> > I'd prefer avoiding this. If the desktop file is sane enough and already
> > provided, it's fine to install it directly.
> 
> By using this command you make the desktop file available on the GNOME/KDE/SFCE
> menu (and/or panel).

Hum, I really don't think desktop-file-install has aything to do with this.
Maybe you're thinking of other tools like update-desktop-database and others,
run from scriplets, which do some "magic"?

Comment 20 Xavier Lamien 2007-05-20 13:59:26 UTC
(in reply to comment #19)
surely...^^

Well,

OK - Mock : Built on FC6 en F-7 (i386 and x86_64)
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 field in spec matches
OK - License is LGPL
OK - License match extras packaging policy licenses allowed
OK - License file is included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources SHOULD match upstream md5sum:
c9ce0b7b3519577b5f460b20c42e04c9  elisa-1.6.0.tar.gz
OK - Package has correct buildroot.
OK - BuildRequires isn't redundant.
OK - %build and %install stages is correct and work.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Changelog section is correct.

OK - Should function as described.
OK - Should package latest version

------------------------------------------------
Rpmlint output:
------------------------------------------------
OK - silent on both srpm and rpm.



==============
** APPROVED **
==============

Comment 21 Matthias Saou 2007-05-21 08:47:52 UTC
New Package CVS Request
=======================
Package Name: elisa
Short Description: Media Center
Owners: matthias@rpmforge.net
Branches: devel F-7 FC-6 EL-5
InitialCC: 

Comment 22 Matthias Saou 2007-05-22 08:34:57 UTC
The devel branch has been rebuilt, others are following now :-)

Comment 23 Alex Lancaster 2009-10-25 19:40:37 UTC
By the way, this package is being re-reviewed for a name from elisa->moovida at bug #530021.


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