Bug 1065745

Summary: Review Request: lltag - tag music files comfortably
Product: [Fedora] Fedora Reporter: Mark Meyer <ofosos>
Component: Package ReviewAssignee: William Moreno <williamjmorenor>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: htl10, leon, ofosos, package-review, volker27, williamjmorenor
Target Milestone: ---Flags: williamjmorenor: fedora-review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: NotReady
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-01-19 14:28:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 177841    

Description Mark Meyer 2014-02-16 16:21:20 UTC
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

Comment 1 Mark Meyer 2014-02-16 16:39:55 UTC
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.

Comment 2 Leon Weber 2014-02-16 16:56:44 UTC
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.

Comment 3 Leon Weber 2014-02-16 17:08:37 UTC
(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.

Comment 4 Mark Meyer 2014-02-16 17:38:04 UTC
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.

Comment 5 Mark Meyer 2014-02-16 17:41:44 UTC
Dang. I left out the changelog, it's updated in the spec file.

Comment 6 Leon Weber 2014-02-16 17:52:10 UTC
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-%{version}.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.

Comment 7 Mark Meyer 2014-02-16 18:13:23 UTC
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

Comment 8 Mark Meyer 2014-02-16 18:20:52 UTC
This should be http://ofosos.org/lltag-0.14.4-6.fc20.src.rpm and the make issue is also resolved.

Comment 9 Leon Weber 2014-02-16 19:05:17 UTC
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).

Comment 10 Christopher Meng 2014-02-17 01:00:28 UTC
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.

Comment 11 Volker Fröhlich 2014-04-22 21:07:00 UTC
Mark, are you still interested?

Comment 12 Hin-Tak Leung 2014-10-07 22:24:28 UTC
Adding cc. - found myself needing it recently.

Comment 13 William Moreno 2017-12-16 04:11:54 UTC
Hello I can sponsor you as packager if you still want to go ahead with this review.

Comment 14 Mark Meyer 2017-12-18 17:15:14 UTC
Yes, I'm still interested. I'll review the packaging guidelines and send an updated request during this week. Thanks for sponsoring me.

Comment 15 William Moreno 2018-03-16 16:52:29 UTC
Any update  here? If you do not post a update soon I will close this ticket as a DEADREVIEW

Comment 16 William Moreno 2018-06-26 16:23:05 UTC
Final ping

Comment 17 Volker Fröhlich 2020-01-19 14:28:40 UTC
I take the liberty to do that.

Comment 18 Red Hat Bugzilla 2023-09-14 02:03:43 UTC
The needinfo request[s] on this closed bug have been removed as they have been unresolved for 1000 days