Bug 382101

Summary: Review Request: picard - MusicBrainz music file tagger
Product: [Fedora] Fedora Reporter: Alex Lancaster <alexl>
Component: Package ReviewAssignee: Nicolas Chauvet (kwizart) <kwizart>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, rdieter
Target Milestone: ---Flags: kwizart: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-11-16 21:38:59 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 320171    
Bug Blocks:    

Description Alex Lancaster 2007-11-14 05:45:30 EST
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

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

This depends has a BuildRequires on a pending release of PyQt4-4.3.x (bug #320171)
Comment 1 Nicolas Chauvet (kwizart) 2007-11-14 05:56:28 EST
starting review
Comment 2 Alex Lancaster 2007-11-14 05:57:23 EST
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 07:46:08 EST
build is failing for dist-f9 (waiting for pyQT4 to rich repository to test for F-8)
Comment 4 Alex Lancaster 2007-11-14 08:28:18 EST
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@users.sourceforge.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 08:54:14 EST
* Wed Nov 14 2007 Alex Lancaster <alexl@users.sourceforge.net> 0.9.0-0.3.beta1
- Create pixmaps directory

Spec: http://alexlan.fedorapeople.org/reviews/picard.spec, 
Comment 6 Nicolas Chauvet (kwizart) 2007-11-14 09:32:19 EST
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
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
extracting debug info from
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 17:09:28 EST
(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 17:12:51 EST
I suspect you meant: bug #248308.
Comment 9 Alex Lancaster 2007-11-14 17:18:37 EST
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 17:26:24 EST
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-14 19:41:42 EST
(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-14 19:59:49 EST
Yes but please create a new review request...
Do not uses an existing bug...
Comment 13 Alex Lancaster 2007-11-14 22:14:38 EST
(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 04:24:42 EST
Package review of libdiscid filed: bug #384191
Comment 15 Alex Lancaster 2007-11-15 05:09:24 EST
* Wed Nov 15 2007 Alex Lancaster <alexl@users.sourceforge.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 07:47:38 EST

    This package (picard) is APPROVED by me

Comment 17 Alex Lancaster 2007-11-16 07:55:28 EST
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 12:13:42 EST
cvs done.
Comment 19 Alex Lancaster 2007-11-16 21:38:59 EST
Builds OK in devel/


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