[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: xinput-calibrator

Hey Paul,
I've just released a new upstream version making some of the issues irrelevant. More comments inline. 

On 08/15/2010 04:58 AM, Paul Wise wrote:

On Sun, Aug 15, 2010 at 12:04 AM, Tias<tias@ulyssis.org>  wrote:


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.


Fair enough.


http://dep.debian.net/deps/dep3/


Done that for now (I hope properly).


debian/patches/debian-changes-0.7.0-3 reverts all the other patches.
Hmm, I don't know why quilt would have created that. However, the patches are incorporated in the new release, so the use of quilt is no longer needed. 


spelling_fixes.patch and typo_traditional.patch can be merged. The
resulting patch should use Applied-Upstream instead of Forwarded.


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 ?


Using dh-autoreconf would mean build-depending on it. Just leave it I
guess. You seem to have dropped this patch now though? Ideally you
would merge this patch into link_as_needed.patch instead of making it
separate.


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.


Ughh, talk about misfeature. I've sent them a mail about it.
Actually I think it is positives, projects on github have a far more descriptive README (and thus documentation) then many other (starting) projects. 


I've removed it using quilt for now.


Removing the debian/docs file is a better way. If I delete
debian-changes-0.7.0-3 and remove it from debian/patches/series, then
the package FTBFS since README no longer exists.


Woops, fixed.


The upstream ChangeLog file should be named NEWS, ChangeLog is
supposed to be more like a VCS log. You can generate a ChangeLog at
tarball build time using git2cl in Makefile.am:

http://josefsson.org/git2cl/

dist-hook:
         LC_ALL=C git log --pretty --numstat --summary $(VERSION) |
git2cl>  $(distdir)/ChangeLog
The changelog consists of all the changesets that went in the release, instead of all the individual changes. I see no point in copying the vcs history, that seems excessive and uninformative to me. I would prefer to keep the changelog in its current format. 


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 ?


It is possible, add another binary stanza in debian/control and run
the build process in two different directories. I would recommend
xinput-calibrator-x11 and xinput-calibrator-gtkmm. I've never done
that with debhelper 7 but it should just be a matter of overriding
dh_auto_* to run them in two different dirs.


TODO


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


That makes sense I guess.


The lintian of my pbuilder seems less stringent : /


I'm using this hook to run lintian in pbuilder:

#!/bin/sh
echo -n 'Installing lintian: '
apt-get install -y --force-yes --no-install-recommends lintian libwww-perl
lintian --info --display-info --display-experimental --pedantic
--show-overrides --checksums --color auto /tmp/buildd/*.changes
exit 0


Thanks, I've added it too.


More issues:

CC-BY-SA 2.5 is non-free so the icon cannot go into the source or
binary package.
Hmm, I took the license from the COPYING file, but their debian/copyright file lists it as CC-BY-SA 3.0, which is DFSG compliant I read.
Ah, apparently they discovered the mismatch too and updated the COPYING file: 
http://bazaar.launchpad.net/~ubuntu-art-pkg/human-icon-theme/ubuntu/revision/103

I've updated the LICENSE file accordingly.



debian/copyright also misses the copyright/license
information for the icon.
added CC-BY-SA 3.0 reference, or do I have to copy the entire 6 page legal code... 


In Makefile.am you should use dist_man_MANS instead of EXTRA_DIST.
I have no idea why or what the difference is (and couldn't find it using simple searchers), but done. 


Shouldn't scripts/Makefile.am install the SVG, XPM and .desktop files?


Absolutely, fixed.


debian/copyright does not document the other copyright holder.
I don't understand this remark, I am the sole main copyright holder. Certain portions were contributed by other developer, hence their copyright is on those specific files too. 



Sven Mueller (in CC:) was already so kind to upload the package on Friday,
how should we proceed next ?


Make an update to fix the stuff above and I'll upload it.


I've upload a new package on mentors.
http://mentors.debian.net/cgi-bin/sponsor-pkglist?action=details;package=xinput-calibrator

I'll look into creating 2 binary packages next.


Thanks and kind regards,
Tias
Reply to: