Bug 233691 - Review Request: clutter - Open Source software library for creating rich graphical user interfaces
Review Request: clutter - Open Source software library for creating rich grap...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: David Moore
Fedora Package Reviews List
:
Depends On:
Blocks: 233695
  Show dependency treegraph
 
Reported: 2007-03-23 15:44 EDT by Allisson Azevedo
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-28 12:42:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
david.moore: fedora‑review+
petersen: fedora‑cvs+


Attachments (Terms of Use)
mock build log of clutter-0.2.2-2 on FC-devel i386 (37.87 KB, text/plain)
2007-03-25 15:37 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description Allisson Azevedo 2007-03-23 15:44:16 EDT
Spec URL: http://fedora.allisson.eti.br/clutter/clutter.spec
SRPM URL: http://fedora.allisson.eti.br/clutter/clutter-0.2.2-1.src.rpm

Description: Clutter is an open source software library for creating fast, 
visually rich graphical user interfaces. The most obvious example 
of potential usage is in media center type applications. 
We hope however it can be used for a lot more.
Comment 1 David Moore 2007-03-24 02:18:20 EDT
I am not yet a Fedora package maintainer, so this review is not official, but
here is my "pre-review":

* %{_libdir}/libclutter-0.2.la should not be included in the package by removing
it after install.

* The INSTALL file should not appear in the documentation since the package is
already built.

* Consider adding:
INSTALL="%{__install} -c -p"
to the make install command line so that timestamps are preserved

* I suggest putting the gtk-doc docs in the -devel package instead of base
because users won't need these unless they are actually developing.  I'm not
sure if that's a written Fedora policy though.

Comment 2 Allisson Azevedo 2007-03-24 12:15:16 EDT
Update package:

Spec URL: http://fedora.allisson.eti.br/clutter/clutter.spec
SRPM URL: http://fedora.allisson.eti.br/clutter/clutter-0.2.2-2.src.rpm
Comment 3 Mamoru TASAKA 2007-03-25 15:37:45 EDT
Created attachment 150861 [details]
mock build log of clutter-0.2.2-2 on FC-devel i386

Very very quick comment
(well, I am currently reviewing more than 10 bugs and
 so please don't expect that I can review this package
 soon... I hope someone else would review this...)

* BuildRequires
  - mockbuild failed as attached. Please add
    the needed BuildRequires.

* Timestamps
  - Keep timestamps on files which are directly
    installed from tarball (header files, html, etc..)
    For this package, the following works
--------------------------------------------
make DESTDIR=$RPM_BUILD_ROOT install INSTALL="%{__install} -p"
--------------------------------------------

* Requires
  - Check the Requires for -devel package.
  * For header files, search the "#include" words
    and add the needed packages as Requires
    accordingly
    E.g. 
-----------------------------------
#include <glib-object.h>
-----------------------------------
    means that this header file should require
    glib2-devel
  - Also, check the "Requires" entry in .pc file.
-----------------------------------
Requires: pangoft2 glib-2.0 >= 2.8 gthread-2.0 gdk-pixbuf-2.0
gdk-pixbuf-xlib-2.0
------------------------------------
     requires some correspoinding packages.
Comment 4 David Moore 2007-03-27 00:12:18 EDT
I'll take this review officially now that I'm a maintainer.  I will do another
check after Mamoru's comments are addressed.
Comment 5 Allisson Azevedo 2007-03-27 08:33:44 EDT
Update package:

Spec URL: http://fedora.allisson.eti.br/clutter/clutter.spec
SRPM URL: http://fedora.allisson.eti.br/clutter/clutter-0.2.2-3.src.rpm

Comment 6 David Moore 2007-03-27 12:38:04 EDT
Good:
 - Package meets naming and packaging guidelines
 - Spec file matches base package name.
 - Spec has consistant macro usage.
 - Meets Packaging Guidelines.
 - License LGPL
 - License field in spec matches
 - License file included in package
 - Spec in American English
 - Spec is legible.
 - Sources match upstream md5sum:
ee2d38920432eb11172e247c471e195c  clutter-0.2.2.tar.gz
 - BuildRequires correct
 - Package has %defattr and permissions on files is good.
 - Package has a correct %clean section.
 - Package has correct buildroot
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 - Package is code or permissible content.
 - Doc subpackage not needed.
 - Packages %doc files don't affect runtime.
 - Headers/static libs in -devel subpackage.
 - Spec has needed ldconfig in post and postun
 - .pc files in -devel subpackage/requires pkgconfig
 - .so files in -devel subpackage.
 - -devel package Requires: %{name} = %{version}-%{release}
 - .la files are removed.
 - Package compiles and builds on at least one arch.
 - Package has no duplicate files in %files.
 - Package doesn't own any directories other packages own.
 - Package owns all the directories it creates.
 - No rpmlint output.
 - final provides and requires are sane.

Bad:
 - There should be a blank line after each entry in %changelog

Please fix that final issue when you commit.  This package is:

APPROVED
Comment 7 Allisson Azevedo 2007-03-27 12:59:09 EDT
New Package CVS Request
=======================
Package Name: clutter
Short Description: OSS library for creating rich gui
Owners: allisson@gmail.com
Branches: FC-6 devel
Comment 8 Jens Petersen 2007-03-28 04:07:06 EDT
done
Comment 9 Mamoru TASAKA 2007-03-28 20:08:45 EDT
Well,
* would you recheck Requires/BuildRequires, especially
  adding "gdk-pixbuf-devel" (this is gtk 1, not gtk2)
  to (Build)Requires is correct?

  NOTE: /usr/include/clutter-0.2/clutter/clutter-stage.h
        contains the line
-------------------------------------------------
    35  #include <gdk-pixbuf/gdk-pixbuf.h>
-------------------------------------------------
        Perhaps this is not gdk-pixbuf-devel, but gtk2-devel
        (/usr/include/gtk-2.0/gdk-pixbuf/gdk-pixbuf.h)

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