Bug 382101 - Review Request: picard - MusicBrainz music file tagger
Summary: Review Request: picard - MusicBrainz music file tagger
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nicolas Chauvet (kwizart)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 320171
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-11-14 10:45 UTC by Alex Lancaster
Modified: 2007-11-30 22:12 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-11-17 02:38:59 UTC
Type: ---
Embargoed:
kwizart: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Alex Lancaster 2007-11-14 10:45:30 UTC
Spec URL: http://alexlan.fedorapeople.org/reviews/picard.spec
SRPM URL: http://alexlan.fedorapeople.org/reviews/picard-0.9.0-0.1.beta1.fc7.src.rpm
Description: 

Picard is an audio tagging application using data from the MusicBrainz
database. The tagger is album or release oriented, rather than
track-oriented.

This depends has a BuildRequires on a pending release of PyQt4-4.3.x (bug #320171)

Comment 1 Nicolas Chauvet (kwizart) 2007-11-14 10:56:28 UTC
starting review

Comment 2 Alex Lancaster 2007-11-14 10:57:23 UTC
Note that "stable" 0.7.x is on the way out, and won't work because it depends on
a version of wxWidgets that Fedora doesn't have any more.

Comment 3 Nicolas Chauvet (kwizart) 2007-11-14 12:46:08 UTC
build is failing for dist-f9 (waiting for pyQT4 to rich repository to test for F-8)
http://koji.fedoraproject.org/koji/taskinfo?taskID=241163


Comment 4 Alex Lancaster 2007-11-14 13:28:18 UTC
New spec: http://alexlan.fedorapeople.org/reviews/picard.spec, 
SRPM: http://alexlan.fedorapeople.org/reviews/picard-0.9.0-0.2.beta1.fc7.src.rpm 

* Wed Nov 14 2007 Alex Lancaster <alexl.net> 0.9.0-0.2.beta1
- Missing BR: python-devel
- Use sitearch to make sure x86_64 builds work
- Install icons share/pixmaps/, rather than share/icons/

Comment 5 Alex Lancaster 2007-11-14 13:54:14 UTC
* Wed Nov 14 2007 Alex Lancaster <alexl.net> 0.9.0-0.3.beta1
- Create pixmaps directory

Spec: http://alexlan.fedorapeople.org/reviews/picard.spec, 
SRPM://alexlan.fedorapeople.org/reviews/picard-0.9.0-0.3.beta1.fc7.src.rpm

Comment 6 Nicolas Chauvet (kwizart) 2007-11-14 14:32:19 UTC
Ok here is a full review:

* License (GPLv2+) COPYING provided - OK
* Checks for License of dependents packages - OK
  for libdiscid "pre"-review https://bugzilla.redhat.com/382101
* rpmlint is clean on rpm - OK (not checked on installed RPM yet)
* Spec is clean - NeedWork (minor)
  --delete-original --remove-category="Application"                             \
 Might change to the same line for the "\"

* Timestamp - NeedWork (minor)
 It is desirable to prevent timestamp changes:
 - when you download source, uses "wget -N "to prevent timestamp change of the
source tarball.
 - When you move files please prevent timestamp to change by using
      install -pm 0644 $RPM_BUILD_ROOT%{_datadir}/icons/picard-*.png
$RPM_BUILD_ROOT%{_datadir}/pixmaps/
then remove old directory with : 
    rm -rf $RPM_BUILD_ROOT%{_datadir}/icons

* Package is built with $RPM_OPT_FLAGS - OK
* Debuginfos are extracted - OK
extracting debug info from
/var/tmp/picard-0.9.0-0.2.beta1.fc9-root-kojibuilder/usr/lib64/python2.5/site-packages/picard/util/astrcmp.so
extracting debug info from
/var/tmp/picard-0.9.0-0.2.beta1.fc9-root-kojibuilder/usr/lib64/python2.5/site-packages/picard/musicdns/ofa.so
13 blocks
* Runtime test - Not tested yet..

See if we could have libdiscid reviewed and imported first. I've asked on the
bug if someone what to submit it for review but as there is already a spec
attached I think you could also propose to take it...
The issues with timestamp are minors, but they are also easy to fix, so please
consider adding support for libdiscid first then update the new picard src.rpm
with fixed timestamp.





Comment 7 Alex Lancaster 2007-11-14 22:09:28 UTC
(In reply to comment #6)
>   for libdiscid "pre"-review https://bugzilla.redhat.com/382101

That just links back to this current bug?  Is there another bug number you
intended to post?


Comment 8 Alex Lancaster 2007-11-14 22:12:51 UTC
I suspect you meant: bug #248308.

Comment 9 Alex Lancaster 2007-11-14 22:18:37 UTC
I had planned to get picard in to Fedora, then submit libdiscid for review after
approval (since it was an optional feature), but if you prefer, I can submit
libdiscid as a new package (based on the spec file on  bug #248308) if you were
willing to review it as well as picard.

Comment 10 Nicolas Chauvet (kwizart) 2007-11-14 22:26:24 UTC
yes! that would be better to have it first, then we might push them together to
F-7 and F-8 stable (if libdiscid is approved)

So add me to the cc for the next review...

Comment 11 Alex Lancaster 2007-11-15 00:41:42 UTC
(In reply to comment #10)
> yes! that would be better to have it first, then we might push them together to
> F-7 and F-8 stable (if libdiscid is approved)
> 
> So add me to the cc for the next review...

Are you will to actually do the review?  (That would motivate me to package it
ASAP ;-) ).


Comment 12 Nicolas Chauvet (kwizart) 2007-11-15 00:59:49 UTC
Yes but please create a new review request...
Do not uses an existing bug...

Comment 13 Alex Lancaster 2007-11-15 03:14:38 UTC
(In reply to comment #12)
> Yes but please create a new review request... Do not uses an existing bug...

Absolutely, that was my plan...  ;) I'll try and submit the libdiscid review
later (06:00 - 10:00 UTC)  so that we can really get this through quickly.  What
timezone are you in?


Comment 14 Alex Lancaster 2007-11-15 09:24:42 UTC
Package review of libdiscid filed: bug #384191

Comment 15 Alex Lancaster 2007-11-15 10:09:24 UTC
* Wed Nov 15 2007 Alex Lancaster <alexl.net> 0.9.0-0.4.beta1
- Various minor spec file cleanups to make sure timestamps stay correct

Spec: http://alexlan.fedorapeople.org/reviews/picard.spec
SRPM: http://alexlan.fedorapeople.org/reviews/picard-0.9.0-0.4.beta1.fc7.src.rpm

Note that as discussed on bug #248308, libdiscid is not a direct dependency for
picard, but it will be pulled in by python-musicbrainz2, which picard does
directly Require.  Since the package is functional without the libdiscid support
(it does not attempt to find libdiscid at build-time), the lack libdiscid should
not prevent this package from being approved and built (hence I'm removing those
bugs related to libdiscid as blockers of this bug).  

However, once python-musicbrainz2 does have libdiscid added as a "Requires",
picard will be able to use the functionality via python-musicbrainz2 at run-time.

Comment 16 Nicolas Chauvet (kwizart) 2007-11-16 12:47:38 UTC
Nice
---------------------------------------------------------------

    This package (picard) is APPROVED by me

---------------------------------------------------------------

Comment 17 Alex Lancaster 2007-11-16 12:55:28 UTC
New Package CVS Request
=======================
Package Name: picard
Short Description: MusicBrainz music file tagger
Owners: alexlan
Branches: F-7 F-8



Comment 18 Kevin Fenzi 2007-11-16 17:13:42 UTC
cvs done.

Comment 19 Alex Lancaster 2007-11-17 02:38:59 UTC
Builds OK in devel/

http://koji.fedoraproject.org/koji/buildinfo?buildID=24624

Will build for F-7 and F-8 once PyQt4-4.3.x goes to stable (bug #320171).


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