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

Re: Re: RFS: xinput-calibrator



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.

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.

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

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

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

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

More issues:

CC-BY-SA 2.5 is non-free so the icon cannot go into the source or
binary package. debian/copyright also misses the copyright/license
information for the icon.

In Makefile.am you should use dist_man_MANS instead of EXTRA_DIST.

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

debian/copyright does not document the other copyright holder.

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

-- 
bye,
pabs

http://wiki.debian.org/PaulWise


Reply to: