Bug 571019 - Review Request: libmtag - An advanced C music tagging library with a simple API
Summary: Review Request: libmtag - An advanced C music tagging library with a simple API
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-03-06 11:26 UTC by Felipe Contreras
Modified: 2012-06-08 15:32 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-06-08 15:32:10 UTC
Type: ---
Embargoed:
hdegoede: fedora-review+


Attachments (Terms of Use)

Description Felipe Contreras 2010-03-06 11:26:56 UTC
Spec URL: http://people.freedesktop.org/~felipec/fedora/libmtag.spec
SRPM URL:
http://people.freedesktop.org/~felipec/fedora/libmtag-0.3.5-1.fc12.src.rpm
Description: libmtag is essentially wrapping the taglib API into a useful C
library, plus providing a bit more functionality.

Here's the koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2034577

This is a continuation of bug #529496.

Comment 1 Felipe Contreras 2010-06-04 21:57:31 UTC
Hello?

Comment 2 Felipe Contreras 2010-08-09 19:19:17 UTC
This package has all the issues fixed.

Comment 3 Hans de Goede 2010-11-03 12:38:15 UTC
Hi Felipe,

http://people.freedesktop.org/~felipec/fedora/libmtag.spec

Gives a 404 error...

Also may I ask why you are packaging libmtag? As it is a library mostly, I assume
you're packaging it for use in something else? I'm asking because for sponsering I always find it good when a submitter submits multiple packages in one go, this gives me a better chance to get a feeling for how well you know the packaging guidelines.

Regards,

Hans

Comment 4 Bug Zapper 2010-11-03 20:39:59 UTC
This message is a reminder that Fedora 12 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 12.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 12 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 5 Felipe Contreras 2010-11-04 11:16:49 UTC
(In reply to comment #3)
> http://people.freedesktop.org/~felipec/fedora/libmtag.spec
> 
> Gives a 404 error...

Fixed.

> Also may I ask why you are packaging libmtag? As it is a library mostly, I
> assume
> you're packaging it for use in something else? I'm asking because for
> sponsering I always find it good when a submitter submits multiple packages in
> one go, this gives me a better chance to get a feeling for how well you know
> the packaging guidelines.

The library is good in itself. The reason I'm packaging it is so I and other people don't have to manually compile it every time.

I know the packaging guidelines for C libraries, but I have never tried python or ruby packaging which is how most people are using this library. That's why I decided to start with the library first.

To be honest, the process to submit packages was painful and demotivational, and after one year and addressing every comment, there's no light on the end of the tunnel.

I guess I'll better give up, write compelling UI applications that use this library, and leave to other people to push for this library, when presumably you guys would bend over due to public demand.

FTR. This package follows the guidelines already.

Comment 6 Hans de Goede 2010-11-04 15:06:17 UTC
(In reply to comment #5)
> (In reply to comment #3)
> > http://people.freedesktop.org/~felipec/fedora/libmtag.spec
> > 
> > Gives a 404 error...
> 
> Fixed.
> 

Thanks.

> > Also may I ask why you are packaging libmtag? As it is a library mostly, I
> > assume
> > you're packaging it for use in something else? I'm asking because for
> > sponsering I always find it good when a submitter submits multiple packages in
> > one go, this gives me a better chance to get a feeling for how well you know
> > the packaging guidelines.
> 
> The library is good in itself. The reason I'm packaging it is so I and other
> people don't have to manually compile it every time.

Ok.

> I know the packaging guidelines for C libraries, but I have never tried python
> or ruby packaging which is how most people are using this library. That's why I
> decided to start with the library first.

Ok.

> To be honest, the process to submit packages was painful and demotivational,
> and after one year and addressing every comment, there's no light on the end of
> the tunnel.

Well it seems you're previous submission ended up in a communications break down / personality clash. I do not want to get into whose fault that is (I don't care), but that does make you're case somewhat special. And I must admit that unfortunately we do not have enough people doing reviews, esp. not reviews of people who need a sponsor.

If you're still interested in becoming a Fedora package maintainer I'm willing to help you through the process.

Step 1 would be for me to review this package, which as you indicate should be a formality. Then I would somehow like to see some more of your packaging skills for example by packaging up a python application using this lib, see here for specific guidelines for python packaging:
http://fedoraproject.org/wiki/Packaging:Python

If that sounds like a good way forward to you let me know, and I'll start by reviewing this package submission.

Comment 7 Felipe Contreras 2010-11-04 21:02:33 UTC
(In reply to comment #6)
> Well it seems you're previous submission ended up in a communications break
> down / personality clash. I do not want to get into whose fault that is (I
> don't care), but that does make you're case somewhat special. And I must admit
> that unfortunately we do not have enough people doing reviews, esp. not reviews
> of people who need a sponsor.
> 
> If you're still interested in becoming a Fedora package maintainer I'm willing
> to help you through the process.
> 
> Step 1 would be for me to review this package, which as you indicate should be
> a formality. Then I would somehow like to see some more of your packaging
> skills for example by packaging up a python application using this lib, see
> here for specific guidelines for python packaging:
> http://fedoraproject.org/wiki/Packaging:Python

I have python bindings, not really a python app.

> If that sounds like a good way forward to you let me know, and I'll start by
> reviewing this package submission.

Ok, I'll put that in my to-do list.

Comment 8 Hans de Goede 2010-11-04 21:58:46 UTC
Hi,

(In reply to comment #7)
> 
> I have python bindings, not really a python app.
> 

python bindings would make a fine second package submission to show some more
of your packaging skills.

I'll go and review this package now, but, once reviewed, I would still like to see something more of your packaging skills before sponsoring you.

Regards,

Hans

Comment 9 Hans de Goede 2010-11-04 22:12:15 UTC
Hmm, it seems the .src.rpm is 404 too. Please fix that, I need to src.rpm to complete the review. In the mean time I've done most of the review based on the .spec file:

Good:
- rpmlint checks return:
libmtag.src: W: spelling-error %description -l en_US taglib -> ta glib, ta-glib, tag lib
libmtag.src: W: spelling-error %description -l en_US backend -> backed, backbend, back end
libmtag.src: W: invalid-url Source0: http://libmtag.googlecode.com/files/libmtag-0.3.5.tar.gz HTTP Error 404: Not Found
libmtag.x86_64: W: spelling-error %description -l en_US taglib -> ta glib, ta-glib, tag lib
libmtag.x86_64: W: spelling-error %description -l en_US backend -> backed, backbend, back end
libmtag-devel.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 6 warnings.

All of these rpmlint warnings are bogus, so this is fine.
- package meets naming guidelines
- package meets packaging guidelines
- license (LGPLv2) OK, text in %doc, matches source
- spec file legible, in am. english
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

Needs work:
- source matches upstream
  I could not verify this because of the lack of a .src.rpm
- summary
  Please drop the An from the summary, as a general rule we try to not have
  Summaries start with A, An or The.
- %doc (main package)
  Please add README and NEWS to the main package %doc, they seem useful
  to have around to me.

Regards,

Hans

Comment 10 Felipe Contreras 2011-02-05 14:52:44 UTC
(In reply to comment #9)
> Hmm, it seems the .src.rpm is 404 too. Please fix that, I need to src.rpm to
> complete the review. In the mean time I've done most of the review based on the
> .spec file:

What's the point of providing the .spec file if it's generated from the .spec and tarball? (both have been provided)

Anyway, libmtag:
http://people.freedesktop.org/~felipec/fedora/libmtag.spec
http://people.freedesktop.org/~felipec/fedora/libmtag-0.3.6-1.fc14.src.rpm

libmtag-python:
http://people.freedesktop.org/~felipec/fedora/libmtag-python.spec
http://people.freedesktop.org/~felipec/fedora/libmtag-python-0.3.1-1.fc14.src.rpm

Here's a koji build for the first, I don't know how to do it for the second:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2763785

> Needs work:
> - source matches upstream
>   I could not verify this because of the lack of a .src.rpm

rpmbuild -ba libmtag.spec

> - summary
>   Please drop the An from the summary, as a general rule we try to not have
>   Summaries start with A, An or The.

Done.

> - %doc (main package)
>   Please add README and NEWS to the main package %doc, they seem useful
>   to have around to me.

Done.

(In reply to comment #8)
> python bindings would make a fine second package submission to show some more
> of your packaging skills.
> 
> I'll go and review this package now, but, once reviewed, I would still like to
> see something more of your packaging skills before sponsoring you.

Done.

Comment 11 Hans de Goede 2011-02-08 09:10:44 UTC
Hi,

(In reply to comment #10)
> (In reply to comment #9)
> > Hmm, it seems the .src.rpm is 404 too. Please fix that, I need to src.rpm to
> > complete the review. In the mean time I've done most of the review based on the
> > .spec file:
> 
> What's the point of providing the .spec file if it's generated from the .spec
> and tarball? (both have been provided)
> 

The src.rpm is what eventually will get imported into the buildsystem / package git. By providing it, you allow the reviewer to verify the tarbal as it will get imported. The reviewer can then verify that this matches the one provided by upstream.

> Anyway, libmtag:
> http://people.freedesktop.org/~felipec/fedora/libmtag.spec
> http://people.freedesktop.org/~felipec/fedora/libmtag-0.3.6-1.fc14.src.rpm
> 

Thanks! Looks good now. I'll approve this as soon as I'm ready to sponsor you
(once the libmtag-python package is also reviewed).

> libmtag-python:
> http://people.freedesktop.org/~felipec/fedora/libmtag-python.spec
> http://people.freedesktop.org/~felipec/fedora/libmtag-python-0.3.1-1.fc14.src.rpm
> 

Thanks, please file a separate review bug for this, using the usual template and let me know the bz number.

Regards,

Hans

Comment 12 Hans de Goede 2011-05-21 20:42:57 UTC
Ping, any update on this / progress with this?

Comment 13 Felipe Contreras 2011-05-22 12:40:03 UTC
(In reply to comment #11)
> > libmtag-python:
> > http://people.freedesktop.org/~felipec/fedora/libmtag-python.spec
> > http://people.freedesktop.org/~felipec/fedora/libmtag-python-0.3.1-1.fc14.src.rpm
> > 
> 
> Thanks, please file a separate review bug for this, using the usual template
> and let me know the bz number.

Here it is:
bug #706705

Comment 14 Hans de Goede 2011-05-29 12:55:35 UTC
Full review done (again):

Good:
- rpmlint checks return:
rpmlint rpmbuild/SRPMS/libmtag-0.3.6-1.fc15.src.rpm rpmbuild/RPMS/x86_64/libmtag-*
libmtag.src: W: spelling-error %description -l en_US taglib -> ta glib, ta-glib, tag lib
libmtag.src: W: spelling-error %description -l en_US backend -> backed, back end, back-end
libmtag.x86_64: W: spelling-error %description -l en_US taglib -> ta glib, ta-glib, tag lib
libmtag.x86_64: W: spelling-error %description -l en_US backend -> backed, back end, back-end
libmtag-devel.x86_64: W: no-documentation
4 packages and 0 specfiles checked; 0 errors, 5 warnings.
 These can all be ignored
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

Approved! I'll sponsor you once python-libmtag is also approved, removing the needsponsor blocker

Comment 15 Hans de Goede 2011-05-29 12:59:06 UTC
Correction to previous comment, the license is LGPLv2, which is mentioned correctly in the spec file.

Comment 16 Felipe Contreras 2011-05-31 22:51:11 UTC
(In reply to comment #15)
> Correction to previous comment, the license is LGPLv2, which is mentioned
> correctly in the spec file.

Huh? Correction to what?

Comment 17 Hans de Goede 2011-06-01 07:47:51 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > Correction to previous comment, the license is LGPLv2, which is mentioned
> > correctly in the spec file.
> 
> Huh? Correction to what?

Sorry for being unclear, correction to *my* previous bugzilla comment, comment #14 in which I wrote:
- license (GPLv2) OK, text in %doc, matches source

Which should have been LGPLv2.

Comment 18 Felipe Contreras 2012-06-08 15:32:10 UTC
I'm not using Fedora any more.

BTW. In Arch Linux doing this takes literally minutes; this low barrier in contributions would ensure that the project will outpace Fedora.


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