[Bug 523523] Review Request: clutter-gesture - Gesture Library for Clutter

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=523523





--- Comment #1 from Christian Krause <chkr at plauener.de>  2009-11-08 16:46:11 EDT ---
Here is the full review of the package:

* rpmlint: ?
rpmlint SRPMS/clutter-gesture-0.0.2-1.fc12.src.rpm RPMS/i686/clutter-gesture-*
SPECS/clutter-gesture.spec 
clutter-gesture-devel.i686: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 1 warnings.

Rpmlint looks clean - in general is a missing documentation acceptable for
devel packages. However, searching for clutter-gesture on moblin.org I've
found a document describing the clutter-gesture API:
http://moblin.org/sites/all/files/Clutter-Gesture-API-0.5.pdf
Depending on its "up-to-date status", would it be an option
to package this as the documentation for the devel package?

* naming: OK
- name matches upstream
- spec file name matches package name

* sources: OK
- md5sum: de0e3e5c01f0a328cc04a46198776ebe  clutter-gesture-0.0.2.tar.bz2
- sources matches upstream
- Source0 tag ok
- spectool -g  works

* URL tag: ?
clutter-gesture has its own project page:
http://moblin.org/projects/clutter-gesture
Would it make sense to use this one?

* License: TODO
- License LGPLv2+ acceptable
- License in spec file does not match the actual license:
In general it does, but one source file (engine/stroke.c) is actual under the
MIT license:
https://fedoraproject.org/wiki/Licensing/MIT#Old_Style_with_legal_disclaimer_2
- So the license in the spec file should be "LGPLv2+ and MIT" (according to
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios
). Please also add a comment about this issue to the spec file.
- License file packaged
- It would be morer clearer if upstream would provide a standard license file
with a proper name like COPYING (and not "NEWS" ;-) ). According to the Review
guidelines the packager is encouraged to query upstream to include it. However
this will not block the review. ;-)

* spec file written in English and legible: OK

* compilation: TODO
- supports parallel build: OK
- RPM_OPT_FLAGS are correctly used: OK
- builds in koji for F13 and F12: OK
- uncommon configure flags: IMHO it shouldn't be necessary to add "--with-pic",
since "--enable-dynamic" is the default anyway
- uncommon CFLAGS: auto*/libtool usually take care of compiling with the
correct
parameters for shared libs
- For testing purposes I've removed "--with-pic" and the CFLAGS definition at
all and according to the output during compilation the library is still
compiled with the correct flags.

* BuildRequires: OK

* locales handling: OK (n/a)

* ldconfig in %post and %postun: OK

* package owns all directories that it creates: OK

* no files listed twice in %files: OK

* file permissions: OK
- %defattr used
- actual permissions in packages ok

* %clean section: OK

* macro usage: OK

* code vs. content: OK (only code)

* large documentation into subpackage: OK (n/a)

* header files in -devel subpackage: OK

* header files: TODO
- clutter-gesture.h includes "config.h" and there is no "config.h" in
/usr/include/clutter-gesture/
- this means, it is not possible to include the public API
- possible solutions:
a) header file should not include "config.h"
b) use an conditional #include directive like
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
or
c) have a "config.h" in the include directory (this should not be done with the
config.h generated by configure)

* static libraries in -static package: OK (n/a)

* package containing *.pc files must "Requires: pkgconfig": OK

* *.so link in -devel package: OK

* devel package requires base package using fully versioned dependency: OK

* packages must not contain *.la files: OK

* GUI applications must provide *.desktop file: OK (n/a)

* packages must not own files/dirs already owned by other packages: OK

* rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK

* all filenames UTF-8: OK

* functional test: ?
How can I test this package? The tests in tests/ just crash if I try
to execute them...

* debuginfo sub-package: OK
- non-empty


Summary
-------

nice to have:
- Package API documentation
- Let URL tag point to specific project page on moblin.org
- What would be a proper functional test?

blocking:
- Add MIT to license tag
- Fix public header files
- Remove unusual %configure flags / CFLAGS

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.