Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: libmtag-python - Simple python bindings for libmtag music tagging library
https://bugzilla.redhat.com/show_bug.cgi?id=706705
Summary: Review Request: libmtag-python - Simple python bindings for libmtag music tagging library Product: Fedora Version: rawhide Platform: Unspecified OS/Version: Unspecified Status: NEW Severity: unspecified Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: felipe.contreras@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora Story Points: ---
Spec URL: http://people.freedesktop.org/~felipec/fedora/libmtag-python.spec SRPM URL: http://people.freedesktop.org/~felipec/fedora/libmtag-python-0.3.1-1.fc14.sr... Description: Bindings for libmtag, a music tagging library which supports: ID3v1, ID3v2 for MP3 files, Ogg Vorbis and FLAC files.
This is a continuation of bug #571019.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
Felipe Contreras felipe.contreras@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Blocks|177841(FE-NEEDSPONSOR) | AssignedTo|nobody@fedoraproject.org |hdegoede@redhat.com Flag| |fedora-review?
--- Comment #1 from Hans de Goede hdegoede@redhat.com 2011-05-29 09:06:10 EDT --- Taking this one, I'll also sponsor Felipe once this review is done
Full review done:
Good: - rpmlint checks return: rpmlint rpmbuild/SRPMS/libmtag-python-0.3.1-1.fc15.src.rpm rpmbuild/RPMS/x86_64/libmtag-python-* libmtag-python.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/libmtag.so libmtag.so()(64bit) 3 packages and 0 specfiles checked; 0 errors, 1 warnings. These can all be ignored - package meets packaging guidelines - 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
Needs work: =========== -package does not meet naming guidelines, python modules should be called python-name rather then name-python, see:
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28p...
So the package (and the specfile) should be named python-libmtag, I realize this contradicts what upstream does, but Fedora names all python modules this way for consistency
-There is no clear license info available in the upstream source tarbal, please ask upstream (I think that may mean asking yourself :) to add a LICENSE file and proper copyright headers to the source files.
-Please include NEWS in the %doc files
-It is ok to include the egginfo file in the Fedora package, see:
http://fedoraproject.org/wiki/Packaging:Python#Packaging_eggs_and_setuptools... But if you consider it not useful to have it is ok to leave it out too
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
Felipe Contreras felipe.contreras@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(hdegoede@redhat.c | |om)
--- Comment #2 from Felipe Contreras felipe.contreras@gmail.com 2011-05-31 18:46:15 EDT --- (In reply to comment #1)
=========== -package does not meet naming guidelines, python modules should be called python-name rather then name-python, see:
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28p...
Error 503 Service Unavailable
So the package (and the specfile) should be named python-libmtag, I realize this contradicts what upstream does, but Fedora names all python modules this way for consistency
Are you sure? There are *a lot* of packages this way:
zinnia-python xapian-bindings-python vtk-python vips-python vigra-python util-vserver-python thunarx-pythonx telepathy-farsight-python stfl-python spice-gtk-python
I could go on.
If I go to the cached page I even see these as examples: * gstreamer-python * gnome-python2 * rpm-python
-There is no clear license info available in the upstream source tarbal, please ask upstream (I think that may mean asking yourself :) to add a LICENSE file and proper copyright headers to the source files.
Copy LGPL v2.1? Ok.
-Please include NEWS in the %doc files
Ok.
-It is ok to include the egginfo file in the Fedora package, see:
http://fedoraproject.org/wiki/Packaging:Python#Packaging_eggs_and_setuptools... But if you consider it not useful to have it is ok to leave it out too
I don't see the point. I'll leave that out.
Please clarify on the naming scheme, it doesn't seem to be strongly enforced.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(hdegoede@redhat.c | |om) |
--- Comment #3 from Hans de Goede hdegoede@redhat.com 2011-06-01 03:46:10 EDT --- Hi,
(In reply to comment #2)
(In reply to comment #1)
=========== -package does not meet naming guidelines, python modules should be called python-name rather then name-python, see:
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28p...
Error 503 Service Unavailable
Works for me, I think you hit a temporary glitch.
So the package (and the specfile) should be named python-libmtag, I realize this contradicts what upstream does, but Fedora names all python modules this way for consistency
Are you sure? There are *a lot* of packages this way:
zinnia-python xapian-bindings-python vtk-python vips-python vigra-python util-vserver-python thunarx-pythonx telepathy-farsight-python stfl-python spice-gtk-python
I could go on.
These are packages where the python bindings are build from the same sources tarbal as the main package, so they follow the usual <main-packagename>-<sub-package-name> convention, however these are the exception if you do: yum list 'python-*' you will find many many more named that wau.
If I go to the cached page I even see these as examples:
- gstreamer-python
- gnome-python2
- rpm-python
-There is no clear license info available in the upstream source tarbal, please ask upstream (I think that may mean asking yourself :) to add a LICENSE file and proper copyright headers to the source files.
Copy LGPL v2.1? Ok.
Yes include a copy of the LGPL v2.1 and add a standard GPL copyright header to the single C file please.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
--- Comment #4 from Felipe Contreras felipe.contreras@gmail.com 2011-06-01 06:32:42 EDT --- (In reply to comment #3)
(In reply to comment #2)
(In reply to comment #1)
So the package (and the specfile) should be named python-libmtag, I realize this contradicts what upstream does, but Fedora names all python modules this way for consistency
Are you sure? There are *a lot* of packages this way:
zinnia-python xapian-bindings-python vtk-python vips-python vigra-python util-vserver-python thunarx-pythonx telepathy-farsight-python stfl-python spice-gtk-python
I could go on.
These are packages where the python bindings are build from the same sources tarbal as the main package, so they follow the usual <main-packagename>-<sub-package-name> convention, however these are the exception if you do: yum list 'python-*' you will find many many more named that wau.
Why would that matter? It certainly doesn't make any different to the end-user how the package was created.
What if I put the bindings inside libmtag, as I actually have been thinking on doing, would I be entitled to name the package libmtag-python then? I don't think that makes sense.
Plus, it's actually not the case on xapian-bindings, thunarx-python, and even gstreamer-python (which is actually mentioned as an example) (and probably others).
If I go to the cached page I even see these as examples:
- gstreamer-python
- gnome-python2
- rpm-python
My feeling is that this is not a strong requirement, if it was, there should be a bug report to rename packages such as gstreamer-python, and it should be take out of the examples.
And if there is indeed such a rule that in-package bindings can be named $NAME-python, it should be mentioned in the documentation. More likely, these packages have to be renamed as well.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=706705
--- Comment #5 from Hans de Goede hdegoede@redhat.com 2011-06-01 06:42:33 EDT --- I did not make the naming rules. The ones which you mention which break them may pre-date the naming rules, or the reviewer was not doing a proper job :)
Anyways I'm fine with making an exception here, esp. considering that at a later date the python bindings may move into the main src tarbal. So lets keep the name as libmtag-python. That just leaves the adding of a copyright header + License file to be sorted out and then we're good to go.
package-review@lists.fedoraproject.org