Bug 384191 (libdiscid)

starting review

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....?

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...)

(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...

Fix tarball timestamp:

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

* 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

* 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

Updated files section in -devel package like so:

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

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

    This package (libdiscid) is APPROVED by me

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

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

cvs done.

Builds fine in devel/ pending updates in updates-testing for F-7 and F-8,
closing review.

Package Change Request
======================
Package Name: libdiscid
New Branches: epel-7
Owners: cicku

Requested at #1065583

Any comments from the Fedora maintiners?

As comaintainer it's obviously fine with me.

(In reply to Jon Ciesla from comment #14)
> Any comments from the Fedora maintiners?

All good with me!

Package Change Request
======================
Package Name: libdiscid
New Branches: epel7
Owners: cicku

Git done (by process-git-requests).