Bug 233695 - Review Request: Pyclutter - Python modules that allow you to use the Clutter toolkit
Summary: Review Request: Pyclutter - Python modules that allow you to use the Clutter ...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Moore
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 233691
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-03-23 20:13 UTC by Allisson Azevedo
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-04-02 02:43:35 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Allisson Azevedo 2007-03-23 20:13:52 UTC
Spec URL: http://fedora.allisson.eti.br/pyclutter/pyclutter.spec
SRPM URL: http://fedora.allisson.eti.br/pyclutter/pyclutter-0.2.0-1.src.rpm

Description: This archive contains the Python modules that allow you to use the
Clutter toolkit in Python programs.

Comment 1 David Moore 2007-03-24 06:42:59 UTC
I am not an official package maintainer yet, but here is my "pre-review":

* You need to change your use of %{python_sitelib} to %{python_sitearch} since
the module is architecture-specific.  This caused the rpmbuild to fail on my
x86_64 for example.  Also, you can remove the unneeded python_sitelib definition
from the top of the file.

* The _clutter.la libtool file should be removed from the install.

* The INSTALL file should be removed.

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


Comment 3 David Moore 2007-03-27 04:15:31 UTC
I'll take this officially now.  I will review this again after bug 233691 is
building in mock.

Comment 4 David Moore 2007-03-28 01:52:41 UTC
Good:
 - Package meets naming and packaging guidelines
 - Spec file matches base package name.
 - Spec has consistant macro usage.
 - Meets Packaging Guidelines.
 - Spec in American English
 - Spec is legible.
 - Sources match upstream md5sum:
cb2565cd0014ccc3f1f2700c8c6f3193  pyclutter-0.2.0.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.
 - Packages %doc files don't affect runtime.
 - Headers/static libs in -devel subpackage.
 - Spec has needed ldconfig in post and postun
 - -devel package Requires: %{name} = %{version}-%{release}
 - 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.
 - No rpmlint output.

Bad:
 - COPYING file contains the GPL but spec says LGPL.  Which is correct?
 - Package should own directory %{python_sitearch}/clutter
 - %changelog should have a blank line after each entry
 - make install should include INSTALL="%{__install} -p" so that timestamps are
preserved.

Comment 6 David Moore 2007-03-28 16:25:23 UTC
Okay, looks good now.

APPROVED

Comment 7 Mamoru TASAKA 2007-03-28 16:35:21 UTC
Sorry, however I switch back to review?

Comment 8 Mamoru TASAKA 2007-03-28 17:13:21 UTC
Well:
The reason why I switched backed to fedora-review? is that
at least the following issues should be fixed before committing
to Fedora.

* Python releated dependency
  - Well, for python related packages, the dependency for it
    should be checked with some more process.
    Usually, this can be checked by what modules the scripts
    in the package try to import.

    Example:
    /usr/lib/python2.5/site-packages/clutter/__init__.py contains the
    line:
-------------------------------------------------------------
     8
     9  # use the pygtk module lazy loading stuff
    10  from gtk._lazyutils import LazyNamespace, LazyModule
    11  
-------------------------------------------------------------
    This means that this package needs "Requires: pygtk2".
    pygtk2 has "/usr/lib/python2.5/site-packages/gtk-2.0/gtk/_lazyutils.py"

* Dependency for -devel packge
  - Again check this. For this, you must check what files
    the header files in this -devel try to include (i.e.
    search "#include" words)

    For pyclutter.h:
-------------------------------------------------------------
     7
     8  #include <glib.h>
     9  
    10  #define NO_IMPORT_PYGOBJECT
    11  #include <pygobject.h>
    12  #include <Python.h>
    13  #include <clutter/clutter.h>
    14  #include <clutter/clutter-feature.h>
    15  
------------------------------------------------------------

* %defatter
  - Please unify. i.e. Use %defattr(-,root,root,-) also to -devel
    package.

* Redundant Requires/BuildRequires
  - Please remove redundant (Build)Requires unless you have special
    reason to leave them.

    python-devel <- this is required by pycairo-devel
                 <- and pycairo-devel is required by pygtk2-devel
    glib2-devel  <- this is required by gtk2-devel
    pygobject2-devel <- this is required by pygtk2-devel

Comment 10 Mamoru TASAKA 2007-03-29 03:52:05 UTC
Well,

For -devel package:

-------------------------------------------------------------
     7
     8  #include <glib.h> <- glib2-devel <- clutter-devel requires this
     9  
    10  #define NO_IMPORT_PYGOBJECT
    11  #include <pygobject.h> <- pygobject2-devel
    12  #include <Python.h> <- python-devel 
    13  #include <clutter/clutter.h> <- clutter-devel
    14  #include <clutter/clutter-feature.h> <- clutter-devel
    15  
------------------------------------------------------------

So Requires for -devel package should be:
------------------------------------------------------------
Requires:       %{name} = %{version}-%{release}
Requires:       clutter-devel
Requires:       python-devel
Requires:       pygobject2-devel
------------------------------------------------------------

Comment 12 Mamoru TASAKA 2007-03-30 16:11:51 UTC
Okay.

Again switching back to fedora-cvs+.

Comment 13 Allisson Azevedo 2007-03-30 16:30:44 UTC
New Package CVS Request
=======================
Package Name: pyclutter
Short Description: Python modules to use the Clutter toolkit
Owners: allisson
Branches: FC-6


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