SPEC: http://pbrobinson.fedorapeople.org/clutter-gesture.spec SRPM: http://pbrobinson.fedorapeople.org/clutter-gesture-0.0.2-1.fc11.src.rpm Description: This library allows clutter applications to be aware of gestures and to easily attach some handlers to the gesture events. The library supports both single and multi touch. koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1681277
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
Fixed: > - Let URL tag point to specific project page on moblin.org > - Add MIT to license tag > - Remove unusual %configure flags / CFLAGS Reported upstream. > - Package API documentation > - Fix public header files eom is the currently only supported test although there should be more support in Moblin 2.2 > - What would be a proper functional test?
Updated SRPM: http://pbrobinson.fedorapeople.org/clutter-gesture-0.0.2-2.fc12.src.rpm Spec location as before.
Thank you very much for the update. (In reply to comment #2) > Fixed: > > - Let URL tag point to specific project page on moblin.org > > - Add MIT to license tag > > - Remove unusual %configure flags / CFLAGS > > Reported upstream. > > - Package API documentation > > - Fix public header files I've reviewed the new package and besides the public header / pkgconfig issue it looks fine now. Have you already heard something from upstream? However, since it is possible to compile eom against clutter-gesture (and the test programs work), the header issue does not block the review. The remaining minor non-blocking issues are: 1. Package API documentation 2. Fix public header files / pkgconfig Please note, that /usr/lib/pkgconfig/clutter-gesture.pc also contains a placeholder which was not replaced by the configure script: modlibexecdir=@modlibexecdir@ This must also be fixed. -> APPROVED
Thanks for the review New Package CVS Request ======================= Package Name: clutter-gesture Short Description: Gesture Library for Clutter Owners: pbrobinson Branches: F-12 InitialCC:
cvs done.
Built and in rawhide