Re: Re: RFS: xinput-calibrator
> On Sun, Aug 8, 2010 at 1:13 AM, Tias <tias@ulyssis.org> wrote:
>
> > I am looking for a sponsor for my package "xinput-calibrator".
> ...
> > I would be glad if someone verified and uploaded this package for me.
>
> Here is a review:
Hey Paul,
Thanks for the detailed review ! Replies inline.
> Since you're upstream it might be a good idea to add a configure
> option to turn on xinput_calibrator_LDFLAGS = -Wl,--as-needed instead
> of using a patch. If not, then please add DEP-3 headers to the patch.
I've applied it upstream but didn't want to do a new upstream release
for just that. I might do one soon with the rest of your suggestions
included though.
> http://dep.debian.net/deps/dep3/
Done that for now (I hope properly).
> debian/patches/debian-changes-0.7.0-2 should probably be replaced by
> using dh-autoreconf to rebuild the autotools stuff.
That seems to be automatically created.
If I use dh-autoreconf then I should build-depend on it too ? Can I just
leave it as it is untill I release a new upstream version, and none of
this will be needed anymore ?
> Your Standards-Version is out of date, please review
> upgrading-checklist.txt from debian-policy and make the appropriate
> changes.
Done.
> Your debian/changelog does not close your ITP.
Done.
> You can remove the comments from debian/rules.
Done.
> Your debian/rules probably doesn't need to run ./autogen.sh since
> release tarballs will always have ./configure in them unless they were
> created in a broken way (i.e. not with `make distcheck`). In any case,
> running it from the pattern rule is the wrong place and you should use
> the override_dh_auto_configure rule.
Yes, that is a leftover from building 'native' packages.
> Usually DH_VERBOSE isn't set in debian/rules.
Removed.
> Please split build instructions from README into README.install since
> they are not useful for users of the binary packages.
>
> Please split changes between releases from README into a NEWS file.
>
> After that, README will only contain information that is duplicated in
> the package description and elsewhere so you can probably just not
> ship it in the Debian package.
I guess that is a side-effect of using github: it shows your 'README'
file on the front page, so it is natural to add quite some information in it
(see http://github.com/tias/xinput_calibrator)
Because of this, I don't intend to remove a lot from it as upstream author.
I don't mind removing the README in the debian package though, the
manpage contains all the information a 'user' needs.
I've removed it using quilt for now.
> I read on the upstream website that it only uses pure X11, but you
> build-depend on GTKmm, why is that? Looking at the debian/changelog
> you seem to regard Debian as a desktop distribution. Debian is a
> universal distribution and is used on everything from phones to
> servers. IMO it would be good to offer both the GTKmm and the pure X11
> versions on Debian.
Yes, that sounds like a good idea.
Is it possible to build 2 binary packages from 1 source ?
The difference would be that the x11 one would not build-depend on gtkmm.
I guess it would also need a different name, perhaps
xinput-calibrator-x11 or xinput-calibrator-native or something ?
> You might want to contact Thibaut GIRKA who is adding support for the
> OpenMoko FreeRunner to the Debian installer. I imagine that
> touch-screen calibration is something that would be useful in d-i/g-i.
Thanks for the reference, I'll contact him.
> There is a spelling error in the code: s/tranditional/traditional/
Fixed.
> Why does the .desktop file run cat?
Its a silly bug in gnome/xfce4-terminal that's already open for 5 years...
https://bugzilla.gnome.org/show_bug.cgi?id=324407
If you open the calibrator from the menu, then the terminal closes
immediatly after the application is finished running. This means that
the user does not see how to make the calibration values permanent. This
is my fix for it...
> Since I have an OpenMoko FreeRunner and use this software on
> non-Debian partitions I'm interested to have it in Debian too.
>
> lintian complaints:
>
> I: xinput-calibrator source: quilt-patch-missing-description
> link_as_needed.patch
> W: xinput-calibrator source: out-of-date-standards-version 3.8.4
> (current is 3.9.1)
> I: xinput-calibrator: spelling-error-in-manpage
> usr/share/man/man1/xinput_calibrator.1.gz usefull useful
> I: xinput-calibrator: spelling-error-in-manpage
> usr/share/man/man1/xinput_calibrator.1.gz Usefull Useful
> I: xinput-calibrator: spelling-error-in-binary
> ./usr/bin/xinput_calibrator Succesfully Successfully
> I: xinput-calibrator: spelling-error-in-binary
> ./usr/bin/xinput_calibrator Succesfully Successfully
> I: xinput-calibrator: spelling-error-in-binary
> ./usr/bin/xinput_calibrator necesary necessary
> I: xinput-calibrator: spelling-error-in-binary
> ./usr/bin/xinput_calibrator necesary necessary
The lintian of my pbuilder seems less stringent : /
All fixed, using quilt because I don't want to rush my other changes for
the next release.
I've uploaded 0.7.0-3 on mentors:
http://mentors.debian.net/cgi-bin/sponsor-pkglist?action=details;package=xinput-calibrator
Sven Mueller (in CC:) was already so kind to upload the package on
Friday, how should we proceed next ?
Thanks again,
Tias
PS. I am not subscribed to the list.
Reply to: