Bug 645277 - Review Request: puddletag - Feature rich, easy to use tag editor
Summary: Review Request: puddletag - Feature rich, easy to use tag editor
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 495324
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-10-21 08:10 UTC by Terje Røsten
Modified: 2011-01-06 19:22 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-06 19:22:12 UTC
Type: ---
Embargoed:
bugs.michael: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
puddletag exception freeze with non-initialized quodlibet (2.22 KB, text/plain)
2010-12-28 09:39 UTC, Michael Schwendt
no flags Details

Description Terje Røsten 2010-10-21 08:10:56 UTC
spec: http://terjeros.fedorapeople.org/puddletag/puddletag.spec
srpm: http://terjeros.fedorapeople.org/puddletag/puddletag-0.9.6-1.fc12.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2546420
Description:

Puddletag is a audio tag editor similar program Mp3tag.

Unlike most taggers, it uses a spreadsheet-like layout so that all the
tags you want to edit by hand are visible and easily editable.

The usual tag editor features are supported like extracting tag
information from filenames, renaming files based on their tags by
using patterns (that you define, not crappy, uneditable ones).

Then there're Functions, which can do things like replace text, trim,
change the case of tags, etc. Actions can automate repetitive
tasks. You can import your QuodLibet library, lookup tags using
MusicBrainz, FreeDB or Amazon (though it's only good for cover art)
and more.

Supported formats: ID3v1, ID3v2 (mp3), MP4 (mp4, m4a, etc.),
VorbisComments (ogg, flac), Musepack (mpc), Monkey's Audio (.ape) and
WavPack (wv).

Comment 2 Michael Schwendt 2010-12-27 18:46:14 UTC
> License:        GPLv2

App's Help dialog says "GPLv2", but multiple source files are GPLv2+. Hence the full tag would be "GPLv2 and GPLv2+":

$ grep "any later version" * -R|grep .py
puddlestuff/tagmodel.py:#(at your option) any later version.
puddlestuff/puddlesettings.py:#(at your option) any later version.
puddlestuff/findfunc.py:#(at your option) any later version.
puddlestuff/actiondlg.py:#(at your option) any later version.
puddlestuff/puddleobjects.py:#(at your option) any later version.
puddlestuff/audioinfo/ogg.py:#(at your option) any later version.
puddlestuff/audioinfo/__init__.py:#(at your option) any later version.
puddlestuff/audioinfo/musepack.py:#(at your option) any later version.
puddlestuff/audioinfo/flac.py:#(at your option) any later version.
puddlestuff/audioinfo/mp4.py:#(at your option) any later version.
puddlestuff/audioinfo/apev2.py:#(at your option) any later version.
puddlestuff/audioinfo/wma.py:#(at your option) any later version.
puddlestuff/audioinfo/wavpack.py:#(at your option) any later version.
puddlestuff/audioinfo/util.py:#(at your option) any later version.
puddlestuff/helperwin.py:#(at your option) any later version.
puddlestuff/webdb.py:#(at your option) any later version.


> URL:            http://puddletag.sourceforge.net

0.9.11 has been released.


> BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Note that several details related to BuildRoot are no longer needed since Fedora 13: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> Requires:       python >= 2.5

This one is wrong actually, since the dep on python(abi) is automatic already and Python is newer than 2.5 for current Fedora dists anyway:

$ rpm -qpR puddletag-0.9.7-1.fc14.noarch.rpm|grep abi
python(abi) = 2.7


> Requires:       PyQt4
> Requires:       pyparsing >= 1.5.5
> Requires:       python-mutagen
> Requires:       python-imaging
> Requires:       python-configobj
> Requires:       python-musicbrainz2 

Even if it may be known that dependencies on Python modules are not automatic, the spec file ought to explain that in a comment:
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

e.g.

# Dependencies on Python modules are not automatic yet.
Requires:       PyQt4
[...]


> %description
> Puddletag is a audio tag editor.

...is an audio tag editor.



> %setup -q
> %{__chmod} 0644 NEWS
> %{__sed} -i -e '1d' \
>    puddlestuff/{webdb,puddlesettings,puddletag,puddleobjects,releasewidget}.py

This sed command is unacceptable IMO. You should at least use a proper match that guards against deleting unexpected changes in those files. Currently, there is a hashbang in those files. You cannot assume that future updates of the package won't be missing the hashbang. In that case you would either delete the UTF-8 encoding or valid code.


> %files
[...]
> %{python_sitelib}/puddlestuff

As a hint, more readable is to add a trailing slash to make clear that this is a directory tree and not a single file:

%{python_sitelib}/puddlestuff/

Comment 3 Terje Røsten 2010-12-27 19:58:44 UTC
Thanks, updated package:
- 0.9.11
- fix license
- add comment about py reqs
- add slash to dir in files
- fix typo in description
- remove buildroot tag
- remove define of python macro
- remove python req
- fix sed expression

spec: http://terjeros.fedorapeople.org/puddletag/puddletag.spec
srpm: http://terjeros.fedorapeople.org/puddletag/puddletag-0.9.11-1.fc14.src.rpm

Comment 4 Terje Røsten 2010-12-27 22:42:07 UTC
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2690582

Comment 5 Michael Schwendt 2010-12-28 09:38:51 UTC
Good.

[...]

I've played a bit more with the run-time aspects of the app and have run into two problems:

First, trouble testing the Music Library feature. A "yum install quodlibet" was necessary to make sense of the "Tools > Import Music Library" menu, which only said:

 | No supported music libraries were found. Most likely the
 | required dependencies aren't installed. Visit the puddletag
 | website, puddletag.sourceforge.net for more details.

Then reselecting the menu and loading QuodLibet made puddletag freeze (will attach the Python exception). Turned out one has to actually run the quodlibet app first to init some files. To reproduce:

 1. rm -rf $HOME/.quodlibet
 2. puddletag
 3. open menu "Tools > Import Music Library"
 4. select QuodLibet and load it

[...]

Second issue was that the preferences default to using "amarok -p" as audio player. On systems where Amarok is not installed (all Fedora non-KDE default installs!), no error/warning dialog is displayed when trying to play a file. If one enters the preferences and removes the default "amarok -p" setting, there is a warning, at least.

I wonder whether there could be a default better than "amarok -p"? Maybe use "xdg-open" (and a dependency on it)?

[...]

Apart from that: APPROVED

Comment 6 Michael Schwendt 2010-12-28 09:39:42 UTC
Created attachment 470930 [details]
puddletag exception freeze with non-initialized quodlibet

Comment 7 Terje Røsten 2010-12-28 21:13:07 UTC
Thanks! I will have a look at these issues before pushing to stable.


New Package SCM Request
=======================
Package Name: puddletag
Short Description: Feature rich, easy to use tag editor
Owners: terjeros
Branches: f13 f14
InitialCC:

Comment 8 Kevin Fenzi 2011-01-02 20:11:40 UTC
Git done (by process-git-requests).

Comment 9 Terje Røsten 2011-01-06 19:22:12 UTC
Imported, built and pushed to testing.


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