Bug 384191 (libdiscid) - Review Request: libdiscid - A library for creating MusicBrainz DiscIDs
Summary: Review Request: libdiscid - A library for creating MusicBrainz DiscIDs
Keywords:
Status: CLOSED NEXTRELEASE
Alias: libdiscid
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nicolas Chauvet (kwizart)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 248308 384251 1065583
TreeView+ depends on / blocked
 
Reported: 2007-11-15 09:23 UTC by Alex Lancaster
Modified: 2014-02-20 13:43 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-11-17 02:35:43 UTC
Type: ---
Embargoed:
kwizart: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Alex Lancaster 2007-11-15 09:23:44 UTC
Spec URL: http://alexlan.fedorapeople.org/reviews/libdiscid.spec
SRPM URL: http://alexlan.fedorapeople.org/reviews/libdiscid-0.1.1-2.fc7.src.rpm
Description: 
libdiscid is a C library for creating MusicBrainz DiscIDs from audio CDs. It
reads a CD's table of contents (TOC) and generates an identifier which can be
used to lookup the CD at MusicBrainz. Additionally, it provides a submission
URL for adding the DiscID to the database.

Comment 1 Nicolas Chauvet (kwizart) 2007-11-15 12:41:17 UTC
starting review

Comment 2 Nicolas Chauvet (kwizart) 2007-11-15 15:53:37 UTC
Well... from README
NOTE: This library is also bundled with libmusicbrainz version 3 or later.
      If you already have libmusicbrainz installed, there is no need to
      install libdiscid separately.

This means the package will be obsoleted soon... So I would say if
libmusicbrainz2 could have libdiscid support, that would be fine to add it from
within the libmusicbrainz2 package and not as a separate package...

If any have futher advices....?




Comment 3 Nicolas Chauvet (kwizart) 2007-11-15 16:03:05 UTC
Well to avoid missunderstanding, the main reason why I think it could be
provided within the libmusicbrainz2 package is that there is no headers in it...
So it is not meant to be used alone... (unless dlopened, but i think not...)

Comment 4 Alex Lancaster 2007-11-16 06:42:30 UTC
(In reply to comment #2)
> Well... from README
> NOTE: This library is also bundled with libmusicbrainz version 3 or later.
>       If you already have libmusicbrainz installed, there is no need to
>       install libdiscid separately.
> 
> This means the package will be obsoleted soon... So I would say if
> libmusicbrainz2 could have libdiscid support, that would be fine to add it from
> within the libmusicbrainz2 package and not as a separate package...
> 
> If any have futher advices....?

We should probably consult the libmusicbrainz maintainer, Bastien and/or the
python-musicbrainz2 maintainer about whether to upgrade libmusicbrainz to version 3.

It's my understanding also that python-musicbrainz2 should use libdiscid, but
whether it's sufficient to have that in libmusicbrainz, or a standalone library,
I'm not sure.  I'll check on the #musicbrainz IRC channel on freenode.

Meanwhile since picard doesn't directly detect or require libdiscid (either at
build-time or run-time), and runs perfectly fine without it, please don't let
this package review stop the picard review from proceeding...



Comment 6 Nicolas Chauvet (kwizart) 2007-11-16 12:02:28 UTC
* Archive file has a better timestamp now - OK

* In Description %{name} is used.. As description need to be a full phrase, we
"usually" begin phrase with a cap...(ie Libdiscid or use This C Library is
for...) - NEED_WORK

* Requires:  pkgconfig is missing in -devel - NEED_WORK

* -devel provide a header into it owns directory, but libdiscid.pc do not
mention this directory...  - SHOULD
This wouldn't cost to have .pc.in patched so it can point to this directory...
Packages that will be built upon this one may fails if they only has #include
<discid.h> - 
What i understand for now is that none app currently directly links to it...:
"this is not a direct wrapper, but contains functions to access libdiscid using
it's own API" - But it would be fine to have it point the right
_includedir/discid if any want to acces it directly later... (I leave this point
up to you..)

* Please save header timestramp to change having : - NEED_WORK
 make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" 
This will prevents multilibs systems to consider the file different has
timestamp will be changed at build time...

* As a COPYING file is present you must include it...  - NEED_WORK


Comment 7 Alex Lancaster 2007-11-16 12:22:54 UTC
* Fri Nov 16 2007 Alex Lancaster <alexl.net> - 0.1.1-5
- Fix description
- devel package Requires: pkgconfig
- save header timestamps

* Fri Nov 16 2007 Alex Lancaster <alexl.net> - 0.1.1-4
- Remove unneeded doc directive in -devel package, add COPYING file

Spec URL: http://alexlan.fedorapeople.org/reviews/libdiscid.spec
SRPM URL: http://alexlan.fedorapeople.org/reviews/libdiscid-0.1.1-5.fc7.src.rpm



Comment 8 Alex Lancaster 2007-11-16 12:31:29 UTC
Updated files section in -devel package like so:

%files devel
%defattr(-,root,root,-)
%{_includedir}/discid/
%{_libdir}/*.so
%{_libdir}/pkgconfig/*.pc

Comment 9 Nicolas Chauvet (kwizart) 2007-11-16 12:36:19 UTC
---------------------------------------------------------------

    This package (libdiscid) is APPROVED by me

---------------------------------------------------------------

Comment 10 Alex Lancaster 2007-11-16 12:44:31 UTC
New Package CVS Request
=======================
Package Name: libdiscid
Short Description: A library for creating MusicBrainz DiscIDs
Owners: alexlan
Branches: F-7 F-8
InitialCC:
Cvsextras Commits: yes


Comment 11 Kevin Fenzi 2007-11-16 17:15:24 UTC
cvs done.

Comment 12 Alex Lancaster 2007-11-17 02:35:43 UTC
Builds fine in devel/ pending updates in updates-testing for F-7 and F-8,
closing review.

Comment 13 Ismael Olea 2014-02-17 10:33:35 UTC
Package Change Request
======================
Package Name: libdiscid
New Branches: epel-7
Owners: cicku

Requested at #1065583

Comment 14 Gwyn Ciesla 2014-02-17 12:57:20 UTC
Any comments from the Fedora maintiners?

Comment 15 Ismael Olea 2014-02-17 13:04:14 UTC
As comaintainer it's obviously fine with me.

Comment 16 Alex Lancaster 2014-02-20 05:24:18 UTC
(In reply to Jon Ciesla from comment #14)
> Any comments from the Fedora maintiners?

All good with me!

Comment 17 Christopher Meng 2014-02-20 05:26:01 UTC
Package Change Request
======================
Package Name: libdiscid
New Branches: epel7
Owners: cicku

Comment 18 Gwyn Ciesla 2014-02-20 13:43:17 UTC
Git done (by process-git-requests).


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