Bug 749320 - Review Request: xinput_calibrator - A generic touchscreen calibration program
Summary: Review Request: xinput_calibrator - A generic touchscreen calibration program
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Martin Gieseking
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-10-26 17:49 UTC by Matthieu Saulnier
Modified: 2011-12-05 09:39 UTC (History)
4 users (show)

Fixed In Version: xinput_calibrator-0.7.5-2.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-26 19:26:23 UTC
Type: ---
Embargoed:
martin.gieseking: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Matthieu Saulnier 2011-10-26 17:49:19 UTC
Spec URL: http://pingou.fedorapeople.org/RPMs/xinput_calibrator.spec
SRPM URL: http://pingou.fedorapeople.org/RPMs/xinput_calibrator-0.7.5-1.fc15.src.rpm
Description: 
xinput_calibrator is a program for calibrating your touchscreen, when using
the X Window System.
It currently features:
 - works for any standard Xorg touchscreen driver (uses XInput protocol)
 - mis-click detection (prevents bogus calibration)
 - dynamically recalibrates the evdev driver
 - outputs the calibration as xorg.conf.d snippet or HAL policy file
 - and more

Hello, this my fourth package, but I haven't a sponsor.

Comment 1 Martin Gieseking 2011-11-04 15:17:47 UTC
Taking the review. Stay tuned.

Comment 2 Martin Gieseking 2011-11-04 19:20:07 UTC
Here's the formal review of your package. It's in pretty good shape. Just a few minor comments: 

- I recommend to use the %{version} macro in Source0 in order to simplify 
  future updates.

- Add the files Changelog and README to the %docs.

- Don't add the .gz suffix to the manpage because the compression format 
  might change. Replace it by an asterisk:
  %{_mandir}/man1/%{name}.1*

- Add INSTALL="install -p" to 'make install' so that the timestamps of the 
  manpage, .desktop file, etc. are preserved.

- If you plan to build the package for EPEL < 6 as well, you must add a 
  BuildRoot field, a %clean section and clean the buildroot at the beginning
  of %install. In case you targeting Fedora and EPEL 6 only, leave everything 
  as is. You might want to remove the %defattr line in %files, though. It's 
  not required for Fedora any longer either.


$ rpmlint /var/lib/mock/fedora-15-x86_64/result/*.rpm
xinput_calibrator.src: W: spelling-error %description -l en_US xinput -> input, x input, Putin
xinput_calibrator.src: W: spelling-error %description -l en_US mis -> mus, mos, mid
xinput_calibrator.src: W: spelling-error %description -l en_US recalibrates -> re calibrates, re-calibrates, calibrates
xinput_calibrator.src: W: spelling-error %description -l en_US evdev -> evade
xinput_calibrator.src: W: spelling-error %description -l en_US xorg -> xor, org, Borg
xinput_calibrator.src: W: spelling-error %description -l en_US conf -> con, cone, cons
xinput_calibrator.x86_64: W: spelling-error %description -l en_US xinput -> input, x input, Putin
xinput_calibrator.x86_64: W: spelling-error %description -l en_US mis -> mus, mos, mid
xinput_calibrator.x86_64: W: spelling-error %description -l en_US recalibrates -> re calibrates, re-calibrates, calibrates
xinput_calibrator.x86_64: W: spelling-error %description -l en_US evdev -> evade
xinput_calibrator.x86_64: W: spelling-error %description -l en_US xorg -> xor, org, Borg
xinput_calibrator.x86_64: W: spelling-error %description -l en_US conf -> con, cone, cons
xinput_calibrator-debuginfo.x86_64: W: spelling-error Summary(en_US) xinput -> input, x input, Putin
xinput_calibrator-debuginfo.x86_64: W: spelling-error %description -l en_US xinput -> input, x input, Putin
3 packages and 0 specfiles checked; 0 errors, 14 warnings.


The above spelling errors are false positive and can be ignored.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - MIT acccording to source file headers

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum xinput_calibrator-0.7.5.tar.gz*
    20da0a2055a5a75962add8c6b44f60fa  xinput_calibrator-0.7.5.tar.gz
    20da0a2055a5a75962add8c6b44f60fa  xinput_calibrator-0.7.5.tar.gz.upstream

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line.
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: If a package installs files below %{_datadir}/icons, the icon cache must be updated.
[.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop file. 
[+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

EPEL <= 5 only:
[X] MUST: The spec file must contain a valid BuildRoot field.
[X] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[X] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.
[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'


[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[X] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[+] SHOULD: Your package should contain man pages for binaries/scripts.

Comment 3 Matthieu Saulnier 2011-11-04 22:05:29 UTC
(In reply to comment #2)
> - I recommend to use the %{version} macro in Source0 in order to simplify 
>   future updates.
Done

> - Add the files Changelog and README to the %docs.
Added

> - Don't add the .gz suffix to the manpage because the compression format 
>   might change. Replace it by an asterisk:
>   %{_mandir}/man1/%{name}.1*
Done

> - Add INSTALL="install -p" to 'make install' so that the timestamps of the 
>   manpage, .desktop file, etc. are preserved.
Added

> - If you plan to build the package for EPEL < 6 as well, you must add a 
>   BuildRoot field, a %clean section and clean the buildroot at the beginning
>   of %install. In case you targeting Fedora and EPEL 6 only, leave everything 
>   as is.
I don't plan to build for EPEL < 6

>   You might want to remove the %defattr line in %files, though. It's 
>   not required for Fedora any longer either.
Removed

Spec URL: http://pingou.fedorapeople.org/RPMs/xinput_calibrator.spec
SRPM URL: http://pingou.fedorapeople.org/RPMs/xinput_calibrator-0.7.5-2.fc15.src.rpm

Comment 4 Martin Gieseking 2011-11-05 12:27:10 UTC
OK, the package looks good and is ready for check-in.

----------------
Package APPROVED
----------------

Comment 5 Matthieu Saulnier 2011-11-05 23:48:04 UTC
New Package SCM Request
=======================
Package Name: xinput_calibrator
Short Description: generic touchscreen calibration program
Owners: fantom
Branches: f15 f16 el6
InitialCC:

Comment 6 Gwyn Ciesla 2011-11-06 20:38:39 UTC
Git done (by process-git-requests).

Comment 7 Fedora Update System 2011-11-08 23:53:37 UTC
xinput_calibrator-0.7.5-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/xinput_calibrator-0.7.5-2.el6

Comment 8 Fedora Update System 2011-11-08 23:56:26 UTC
xinput_calibrator-0.7.5-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/xinput_calibrator-0.7.5-2.fc15

Comment 9 Fedora Update System 2011-11-08 23:58:06 UTC
xinput_calibrator-0.7.5-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/xinput_calibrator-0.7.5-2.fc16

Comment 10 Fedora Update System 2011-11-10 07:31:03 UTC
xinput_calibrator-0.7.5-2.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 11 Fedora Update System 2011-11-26 19:26:23 UTC
xinput_calibrator-0.7.5-2.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 12 Fedora Update System 2011-11-26 23:01:23 UTC
xinput_calibrator-0.7.5-2.fc16 has been pushed to the Fedora 16 stable repository.

Comment 13 Fedora Update System 2011-11-26 23:09:40 UTC
xinput_calibrator-0.7.5-2.fc15 has been pushed to the Fedora 15 stable repository.


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