Bug 384191 - (libdiscid) Review Request: libdiscid - A library for creating MusicBrainz DiscIDs
Review Request: libdiscid - A library for creating MusicBrainz DiscIDs
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nicolas Chauvet (kwizart)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 248308 384251 1065583
  Show dependency treegraph
 
Reported: 2007-11-15 04:23 EST by Alex Lancaster
Modified: 2014-02-20 08:43 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-16 21:35:43 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kwizart: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Alex Lancaster 2007-11-15 04:23:44 EST
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 07:41:17 EST
starting review
Comment 2 Nicolas Chauvet (kwizart) 2007-11-15 10:53:37 EST
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 11:03:05 EST
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 01:42:30 EST
(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 07:02:28 EST
* 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 07:22:54 EST
* Fri Nov 16 2007 Alex Lancaster <alexl@users.sourceforge.net> - 0.1.1-5
- Fix description
- devel package Requires: pkgconfig
- save header timestamps

* Fri Nov 16 2007 Alex Lancaster <alexl@users.sourceforge.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 07:31:29 EST
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 07:36:19 EST
---------------------------------------------------------------

    This package (libdiscid) is APPROVED by me

---------------------------------------------------------------
Comment 10 Alex Lancaster 2007-11-16 07:44:31 EST
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 12:15:24 EST
cvs done.
Comment 12 Alex Lancaster 2007-11-16 21:35:43 EST
Builds fine in devel/ pending updates in updates-testing for F-7 and F-8,
closing review.
Comment 13 Ismael Olea 2014-02-17 05:33:35 EST
Package Change Request
======================
Package Name: libdiscid
New Branches: epel-7
Owners: cicku

Requested at #1065583
Comment 14 Gwyn Ciesla 2014-02-17 07:57:20 EST
Any comments from the Fedora maintiners?
Comment 15 Ismael Olea 2014-02-17 08:04:14 EST
As comaintainer it's obviously fine with me.
Comment 16 Alex Lancaster 2014-02-20 00:24:18 EST
(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 00:26:01 EST
Package Change Request
======================
Package Name: libdiscid
New Branches: epel7
Owners: cicku
Comment 18 Gwyn Ciesla 2014-02-20 08:43:17 EST
Git done (by process-git-requests).

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