https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Bug ID: 1065745 Summary: Review Request: lltag - tag music files comfortably Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: ofosos@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://ofosos.org/lltag.spec SRPM URL: http://ofosos.org/lltag-0.14.4-1.fc20.src.rpm Description: lltag is a frontend to tag (and rename) mp3/ogg/flac files automagically. It uses mp3info, vorbiscomment and metaflac to do so. Fedora Account System Username: ofosos
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #1 from Mark Meyer ofosos@gmail.com --- Please use http://ofosos.org/lltag-0.14.4-2.fc20.src.rpm
The earlier module contained an error putting perl files into /usr/lib/ instead of /usr/share
Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Leon Weber leon@leonweber.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |leon@leonweber.de
--- Comment #2 from Leon Weber leon@leonweber.de --- Some issues (this is an **INFORMAL** review):
- Summary should not end in a dot - License should be GPLv2+ instead of GPL - Source0 should contain the full URL of the source tarball - It would be nicer to format the spec file in a way that all tag values are lined up in one colum - The description field has a trailing whitespace - The %files section should be using macros instead of hardcoded paths - Put upstream’s COPYING, Changes and README files in a %doc directory - %changelog section must have the package version, and should have a (non-empty) entry (e.g. “First version for Fedora”)
Please read http://fedoraproject.org/wiki/How_to_create_an_RPM_package as an introduction and https://fedoraproject.org/wiki/Packaging:Guidelines for full guidelines.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #3 from Leon Weber leon@leonweber.de --- (In reply to Leon Weber from comment #2)
- The description field has a trailing whitespace
What I meant to write was that it has a _leading_ whitespace.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |volker27@gmx.at Whiteboard| |NotReady
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #4 from Mark Meyer ofosos@gmail.com --- Leon, thanks for your feedback! I think I fixed you remarks.
I think I was careful to make the subdirectories under /usr/share/doc, /etc and /usr/share/perl5 owned by the package.
The koji build is here: http://koji.fedoraproject.org/koji/taskinfo?taskID=6535942 Spec: http://ofosos.org/lltag.spec SRPM: http://ofosos.org/lltag-0.14.4-5.fc20.src.rpm
This is my first package so I will need a sponsor.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #5 from Mark Meyer ofosos@gmail.com --- Dang. I left out the changelog, it's updated in the spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #6 from Leon Weber leon@leonweber.de --- You’re mixing tabs and spaces for indention in the spec file, e.g.
8 Source0: http://download.gna.org/lltag/lltag-0.14.4.tar.bz2 9 Patch1: lltag-prefix.patch
Line 8 uses spaces while line 9 uses tabs. That’s why it looks unaligned in my editor. It would be better to keep it consistent.
It’s recommended to put the version tag in the source URL, so that it doesn’t have to be changed with every version bump:
http://download.gna.org/lltag/lltag-%%7Bversion%7D.tar.bz2
(You could even replace lltag with %{name}.)
make doesn’t need to be specified in the BuildRequires (see https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires for a list of exceptions).
Changelog still missing a version tag, see https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs for valid formats.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #7 from Mark Meyer ofosos@gmail.com --- There's an update of the spec file: http://ofosos.org/lltag.spec And a new SRPM: http://ofosos.org/lltag-0.14.4-2.fc20.src.rpm
The alignment issue is fixed. The source tag is fixed. %{version} and %{name} added to the url.
Koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6535955
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #8 from Mark Meyer ofosos@gmail.com --- This should be http://ofosos.org/lltag-0.14.4-6.fc20.src.rpm and the make issue is also resolved.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #9 from Leon Weber leon@leonweber.de --- This fails to build:
error: Installed (but unpackaged) file(s) found: /usr/share/doc/lltag/COPYING /usr/share/doc/lltag/Changes /usr/share/doc/lltag/README
This happens because you copy these files to %{_docdir} in the %install section. In the %files section, however, you don’t reference the files in %{_docdir} but the ones directly in $RPM_BUILD_ROOT. Just drop the cp line and the mkdir from the %install section and you should be good.
Some more minor issues:
The Summary line doesn’t need to repeat the package name (e.g. “A frontend to tag…” instead of “lltag is a frontend to tag…”).
Nitpicking: There’s still _one_ tab character in the spec file, after the colon in line 9, followed by seven space characters. ;-)
Break the description field into multiple lines of <= 80 characters each.
Run rpmlint to test for these things: See http://fedoraproject.org/wiki/How_to_create_an_RPM_package#Test_with_rpmlint and the following sections.
Btw, you should stick to the format from the template for posting links to spec and rpm files, so that the fedora-review tool can pick them up automatically:
Spec URL: http://ofosos.org/lltag.spec SRPM URL: http://ofosos.org/lltag-0.14.4-2.fc20.src.rpm
That way you don’t risk a reviewer accidentally reviewing an old version (fedora-review picks the latest URL it can find).
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Leon Weber leon@leonweber.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
--- Comment #10 from Christopher Meng cickumqt@gmail.com --- Friendly reminder:
Please try to find a sponsor by yourself, don't rely on any people here, we all can't help you.
Please make a self introduction email in -devel list. This may help.
Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ofosos@gmail.com Flags| |needinfo?(ofosos@gmail.com)
--- Comment #11 from Volker Fröhlich volker27@gmx.at --- Mark, are you still interested?
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Hin-Tak Leung htl10@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |htl10@users.sourceforge.net
--- Comment #12 from Hin-Tak Leung htl10@users.sourceforge.net --- Adding cc. - found myself needing it recently.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |williamjmorenor@gmail.com Assignee|nobody@fedoraproject.org |williamjmorenor@gmail.com Flags| |fedora-review?
--- Comment #13 from William Moreno williamjmorenor@gmail.com --- Hello I can sponsor you as packager if you still want to go ahead with this review.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
Mark Meyer ofosos@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ofosos@gmail.com) |
--- Comment #14 from Mark Meyer ofosos@gmail.com --- Yes, I'm still interested. I'll review the packaging guidelines and send an updated request during this week. Thanks for sponsoring me.
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ofosos@gmail.com)
--- Comment #15 from William Moreno williamjmorenor@gmail.com --- Any update here? If you do not post a update soon I will close this ticket as a DEADREVIEW
https://bugzilla.redhat.com/show_bug.cgi?id=1065745
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ofosos@gmail.com)
--- Comment #16 from William Moreno williamjmorenor@gmail.com --- Final ping
package-review@lists.fedoraproject.org