[Bug 509310] Review Request: gpointing-device-settings - Configuration tool for pointing devices

[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=509310





--- Comment #11 from Christian Krause <chkr at plauener.de>  2009-07-24 07:25:34 EDT ---
Here is my full review of the package. In general it looks ok, but there are
some TODO items left. Once there are fixed, please provide again the spec file
as well as a source rpm.

* rpmlint: OK
rpmlint SPECS/gpointing-device-settings.spec
SRPMS/gpointing-device-settings-1.3.1-2.fc11.src.rpm RPMS/i586/libgpds-*
RPMS/i586/gpointing-device-settings-*
libgpds-devel.i586: W: no-documentation
gpointing-device-settings.i586: W: non-conffile-in-etc
/etc/gconf/schemas/gpointing-device-settings_gnome_settings_daemon.schemas
5 packages and 1 specfiles checked; 0 errors, 2 warnings.

both warnings are false positive:
- no-documentation: nothing can be done here
- non-conffile-in-etc for schema files can be ignored, schema files are no
config files

* naming / sub-packaging: TODO
- name matches upstream
- spec file name matches package name
- I suggest to avoid the splitting of the daemon plugin/settings dialog and the
library into separate packages, because of the following reasons:
a) the language files seem to contain the translations for both - it would be
quite strange if e.g. the libgpds package should be installed stand-alone and
there would be no translations available - the other way around (put the
translations into the libgpds package) won't be much better
b) I don't think that the library will be used by other tools for now.

IMHO having just two packages:
gpointing-device-settings
and 
gpointing-device-settings-devel
would be better.

* sources: OK
- spectool -g works
- md5sum 2b0a567739fb565364cdca8dfc72545c 
gpointing-device-settings-1.3.1.tar.gz
- sources matches upstream

* License: OK
- License LGPLv3+ acceptable
- License in spec file matches the actual license 
- License file packaged

* spec file written in English and legible: OK

* compilation: OK
- supports parallel build
- RPM_OPT_FLAGS are correctly used
- builds in mock (F11)
- builds in koji for F11 and F12

* BuildRequires: OK

* locales handling: OK

* 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 sub-package: OK (n/a)

* header files in -devel sub-package: OK

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

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

* pkgconfig file: TODO
- The pkgconfig file contains the following linker flags:
Libs: -L${libdir} -lgpointing-device-settings
which won't work at all, since the library's name is "libgpds.so". Please
substitute the line with something like this:
Libs: -L${libdir} -lgpds

* *.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: TODO
I'm quite sure that the gpointing-device-settings is the regular configuration
dialog for the user to setup his mouse/touchpad preferences. Similar as it was
done in the gsynaptics package it must be available in the Preferences menu:
e.g. in System -> Preferences -> Pointing Device Settings
Please add the according .desktop file.

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

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

* all file names UTF-8: OK

* functional test: OK
- settings dialog basically works
- changing of a couple of settings has the desired effect

* debuginfo sub-package: OK
- non-empty

* Obsoletes: TODO
- Obsoletes: the version must be greater than the version of the current
package, otherwise the current gsynaptics pacakge would not be obsoleted
Obsoletes: gsynaptics < 0.9.16-2

* Scriptlets: TODO
- gconf: please adjust the GConf scriptlets according to
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GConf

-- 
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.