Bug 446158 - Review Request: xesam-glib - A GObject library for dealing with Xesam services
Review Request: xesam-glib - A GObject library for dealing with Xesam services
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-13 00:47 EDT by Deji Akingunola
Modified: 2008-06-28 23:18 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-06-28 23:18:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Deji Akingunola 2008-05-13 00:47:24 EDT
Spec URL: ftp://czar.eas.yorku.ca/pub/xesam/xesam-glib.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/xesam/xesam-glib-0.2.1-1.fc9.src.rpm
Description: Xesam-GLib is a GObject library for dealing with Xesam services.
Comment 1 Jason Tibbitts 2008-06-25 16:35:51 EDT
I have absolutely no idea what Xesam is; could you at least define it in your
%description?

I would suggest using http://xesam.org/people/kamstrup/xesam-glib/ as your URL:
tag; this at least has some information on the package.

rpmlint is quiet except for the following:
  xesam-glib.x86_64: W: unused-direct-shlib-dependency
   /usr/lib64/libxesam-glib.so.0.0.0 /lib64/libdbus-1.so.3
This means that the libxesam-glib links against libdbus-1 but doesn't actually
call any functions from it.  There's a quick libtool tweak that should fix this:
http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues#unused-direct-shlib-dependency
 
I note that 0.3 is out now; I don't see anything that would change this review.

I also noticed that there's a test suite in the source.  A naive "make check"
didn't work for me, though.  Have you looked into whether or not it's runnable?

* source files match upstream:
   8fde51fd248f9215d78c366d5827e39826b2c09007398a05962f4d1d7ab32efd  
   xesam-glib-0.2.1.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
X description could use a definition of Xesam.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
X rpmlint has an issue that should be looked into.
* final provides and requires are sane:
  xesam-glib-0.2.1-1.fc10.x86_64.rpm
   libxesam-glib.so.0()(64bit)
   xesam-glib = 0.2.1-1.fc10
  =
   /sbin/ldconfig
   libdbus-1.so.3()(64bit)
   libdbus-glib-1.so.2()(64bit)
   libglib-2.0.so.0()(64bit)
   libgobject-2.0.so.0()(64bit)
   libxesam-glib.so.0()(64bit)

  xesam-glib-devel-0.2.1-1.fc10.x86_64.rpm
   xesam-glib-devel = 0.2.1-1.fc10
  =
   dbus-glib-devel
   libxesam-glib.so.0()(64bit)
   pkgconfig
   xesam-glib = 0.2.1-1.fc10

X %check is not present, but some sort of test suite is in the tarball.
* shared libraries installed:
  ldconfig called properly.
  unversioned .so files are in the -devel package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets OK (ldconfig).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* pkgconfig file in the -devel package; pkgconfig dependency is present.
* no static libraries.
* no libtool .la files.
Comment 2 Deji Akingunola 2008-06-25 17:55:06 EDT
(In reply to comment #1)
> I have absolutely no idea what Xesam is; could you at least define it in your
> %description?
> 
> I would suggest using http://xesam.org/people/kamstrup/xesam-glib/ as your URL:
> tag; this at least has some information on the package.
> 
> rpmlint is quiet except for the following:
>   xesam-glib.x86_64: W: unused-direct-shlib-dependency
>    /usr/lib64/libxesam-glib.so.0.0.0 /lib64/libdbus-1.so.3
> This means that the libxesam-glib links against libdbus-1 but doesn't actually
> call any functions from it.  There's a quick libtool tweak that should fix this:
>
http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues#unused-direct-shlib-dependency
>  
> I note that 0.3 is out now; I don't see anything that would change this review.
>

I've addressed all the issues you pointed out above.
 
> I also noticed that there's a test suite in the source.  A naive "make check"
> didn't work for me, though.  Have you looked into whether or not it's runnable?
>
One would need a desktop search service that support xesam specifications for
'make check' to work. Only the current versions of strigi and beagle have such
support (to my knowledge); tracker support is coming along.

Thanks for the review.

Spec URL: ftp://czar.eas.yorku.ca/pub/xesam/xesam-glib.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/xesam/xesam-glib-0.3.0-1.fc9.src.rpm
 
Comment 3 Jason Tibbitts 2008-06-26 19:16:23 EDT
I guess we'll skip the test suite, then, although it would to get a comment
about it in the spec.  rpmlint is quiet now and the %description looks good.

The only other nit I can find to pick is that the %description for the -devel
package could use a period at the end.  That's really minor, though.

APPROVED
Comment 4 Deji Akingunola 2008-06-27 10:51:02 EDT
New Package CVS Request
=======================
Package Name: xesam-glib
Short Description: A GObject library for dealing with Xesam services
Owners: deji
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 5 Kevin Fenzi 2008-06-27 12:53:59 EDT
cvs done.
Comment 6 Deji Akingunola 2008-06-28 23:18:00 EDT
I put a comment about skipping 'make check' in the spec. Package now built for
both rawhide and F-9. Thanks.

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