Bug 761063 - Review Request: cover_grabber - Download album cover art
Review Request: cover_grabber - Download album cover art
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
https://sourceforge.net/projects/cove...
NotReady
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2011-12-07 11:14 EST by Jayson Vaughn
Modified: 2015-07-21 09:21 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-07-21 09:21:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jayson Vaughn 2011-12-07 11:14:36 EST
Spec URL: http://69.164.204.114/cover_grabber.spec
SRPM URL: http://69.164.204.114/cover_grabber-0.0.2-1.fc16.src.rpm
Description: Cover Grabber will recursively traverse a specified directory of mp3s
and download album cover art from LastFM.

For instance:
$ covergrabber "/home/user/Music"


This is my first package and I am seeking sponsorship.
Comment 1 Jayson Vaughn 2011-12-07 11:16:10 EST
rpmlint output:

cover_grabber.src: W: spelling-error %description -l en_US covergrabber -> cover grabber, cover-grabber, recoverable
cover_grabber.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cover_grabber/media_dir_walker.py
cover_grabber.noarch: E: incorrect-fsf-address /usr/bin/covergrabber
cover_grabber.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cover_grabber/mp3_handler.py
cover_grabber.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cover_grabber/__init__.py
cover_grabber.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cover_grabber/lastfm_downloader.py
cover_grabber.noarch: W: no-manual-page-for-binary covergrabber
2 packages and 1 specfiles checked; 5 errors, 2 warnings.
Comment 2 Jayson Vaughn 2011-12-07 12:05:47 EST
I had to update GPL license info on all files to get rid of "incorrect-fsf-address" error in rpmlint.  Here is newest rpmlint output:

cover_grabber.src: W: spelling-error %description -l en_US covergrabber -> cover grabber, cover-grabber, recoverable
cover_grabber.noarch: W: no-manual-page-for-binary covergrabber
2 packages and 1 specfiles checked; 0 errors, 2 warnings.
Comment 3 Terje Røsten 2011-12-07 12:38:49 EST
# Remove, not needed at all

%define name cover_grabber
%define version 0.0.2
%define unmangled_version 0.0.2
%define release 1

# You are upstream too?
Source0: http://69.164.204.114/%{name}-%{unmangled_version}.tar.gz

# Remove
Prefix: %{_prefix}

# Remove
Vendor: Jayson Vaughn <vaughn.jayson@gmail.com>

# Remove
URL: http://github.com/thedonvaughn/cover_grabber

Requires: python-mutagen

# Does the tool only work for mp3, not flac et al?
%description
Cover Grabber will recursively traverse a specified directory of mp3s 
and download album cover art from LastFM.

For instance:
$ covergrabber "/home/user/Music"

%prep
%setup -q %{name}-%{unmangled_version}

%build
python setup.py build

%install
# Don't use --record
# Please read this: http://fedoraproject.org/wiki/Packaging:Python
python setup.py install --root=$RPM_BUILD_ROOT --record=INSTALLED_FILES

%clean
rm -rf $RPM_BUILD_ROOT

# use hand built files list.
%files -f INSTALLED_FILES

#  change to %defattr(-,root,root,-)
%defattr(-,root,root)


Please create a FAS account and do a koji scratch build, post link to build here.
Comment 4 Jayson Vaughn 2011-12-07 12:45:46 EST
Thanks Terje.

So I should remove all %define lines?  I use these as variables in my spec file.  Should I just hard code the values instead?

Yes, I am the upstream.

Currently the tool only works on mp3s however I will probably have flac and ogg support added this weekend when I have the time.  Now that I am using python-mutagen, it should be a rather simple refactor.

I already have a FAS account and will perform a koji scratch build shortly.

Thanks again.
Comment 5 Terje Røsten 2011-12-07 12:54:08 EST
Yes, remove all %define, these will be variables any way (just try), expect unmangled_version, however unmangled_version == version any way.
Comment 6 Jayson Vaughn 2011-12-07 14:24:19 EST
Oh, Duh.  I knew these would be variables anyway.

OK I made the changes you mentioned.  Same URLs as above for the new SRPM and new spec file.


Here is the Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3573082

Thanks.
Comment 7 Terje Røsten 2011-12-07 15:59:15 EST
I like line up the tags like this:

Summary:  Download album cover art
Name:     cover_grabber
Version:  0.0.2
Release:  1%{?dist}

you might want that.

Your BuildRoot: tag seems too simple. (Or you can remove it.)


%setup -q cover_grabber-0.0.2

You can drop the cover_grabber-0.0.2 here


Please use the %changelog and increase release when you do changes.
Also innclude links to updated spec and srpm in each update comment.
It's them much simpler for the reviewer to follow the process.

koji build looks good, thanks.
Comment 8 Jayson Vaughn 2011-12-07 16:22:36 EST
Ok,
I've made the suggested changes to the spec file.

I've also added OGG and FLAC support to this application upstream so I re-packaged it.

Here are the new files:

Spec URL: http://69.164.204.114/cover_grabber.spec
SRPM URL: http://69.164.204.114/cover_grabber-1.0.0-1.fc16.src.rpm

rpmlint output:
cover_grabber.src: W: spelling-error %description -l en_US ogg -> egg, org, boggy
cover_grabber.src: W: spelling-error %description -l en_US covergrabber -> cover grabber, cover-grabber, recoverable
cover_grabber.noarch: W: spelling-error %description -l en_US ogg -> egg, org, boggy
cover_grabber.noarch: W: no-manual-page-for-binary covergrabber
2 packages and 1 specfiles checked; 0 errors, 4 warnings.


Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3573245


I really appreciate all your help!
Comment 9 Jayson Vaughn 2011-12-09 11:44:47 EST
Spec URL: http://69.164.204.114/cover_grabber.spec
SRPM URL: http://69.164.204.114/cover_grabber-1.1.0-1.fc16.src.rpm

rpmlint output:

cover_grabber.src: W: spelling-error %description -l en_US ogg -> egg, org, boggy
cover_grabber.src: W: spelling-error %description -l en_US covergrabber -> cover grabber, cover-grabber, recoverable
cover_grabber.noarch: W: spelling-error %description -l en_US ogg -> egg, org, boggy
cover_grabber.noarch: W: no-manual-page-for-binary covergrabber
2 packages and 1 specfiles checked; 0 errors, 4 warnings.


Will post Koji scratch build when Koji is operational again.  Currently it is down due to NFS failure.
Comment 10 Jayson Vaughn 2011-12-12 00:29:50 EST
When koji came back online the builds failed because of lack of python-setuptools (Weird that it worked before the NFS crash).  I have a new release with an updated spec file with 'BuildRequires: python-setuptools' :

-------

Spec URL: http://69.164.204.114/cover_grabber.spec
SRPM URL: http://69.164.204.114/cover_grabber-1.1.1-1.fc16.src.rpm

--------

rpmlint output:

cover_grabber.noarch: W: spelling-error %description -l en_US ogg -> Gog, egg, org
cover_grabber.noarch: W: no-manual-page-for-binary covergrabber
cover_grabber.src: W: spelling-error %description -l en_US ogg -> Gog, egg, org
cover_grabber.src: W: spelling-error %description -l en_US covergrabber -> cover grabber, cover-grabber, overgraze
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

--------

Koji scratch build:

Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=3578806
Comment 11 Jason Tibbitts 2012-06-29 19:09:50 EDT
The URLs don't respond for me; things just time out.  Maybe that's why you aren't getting much response to this ticket.  Why use some random IP address when you could just ask for fedorapeople space?  https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Upload_Your_Package has the details.

Please clear the Whiteboard above if providing a package to review.
Comment 12 Jayson Vaughn 2012-07-03 20:44:58 EDT
My apologies.  I was actually working on a new package as much has changed.

I will also make sure to use my fedorapeople space next time.

Thanks for the suggestions and taking the time to review this ticket.  I will clear the whiteboard when it's ready.
Comment 13 Terje Røsten 2013-11-24 08:37:24 EST
Still any interest or should this request be closed?
Comment 14 Miroslav Suchý 2015-07-21 09:21:18 EDT
Closing due long inactivity. Feel free to reopen if you want to continue.

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