Re: [Tails-dev] Please review and merge feature/6342-update-…

Delete this message

Reply to this message
Author: intrigeri
Date:  
To: The Tails public development discussion list
Subject: Re: [Tails-dev] Please review and merge feature/6342-update-camouflage-for-gnome3
Alan wrote (27 May 2014 17:36:04 GMT) :
> - gnome-theme-windows8 (@git.tails.boum.org)


If there's any Debian packaging in there, please assign the ticket
to me.

> - gnome-panel-applet-window-picker (@git.tails.boum.org). Please review
> C code as I'm not knowledgable enough.


The package builds for me, and Lintian is happy. There are a few
problems, though, including some major ones => a bit more work is
needed. Sadly, most of it would have been avoid if things had been
done right from the beginning. But that's how one learns, I guess :)

This package was converted to a native one. I really don't get why,
and see no indication other than "Update packaging". I doubt we're now
upstream for this code. We don't want to release
window-picker-applet-$UPSTREAM_VERSION.tar.gz, with a different
content, but the same version, as upstream's tarball. Also, I assume
that we'll be happy if Debian or any derivative wants to base their
work on ours, so I think this really ought not to be a non-native one.
That's the top blocker to me.

The changelog entries until 0.5.8-2 (last version that was part of
Debian) were deleted. Same for Ubuntu's changelog entries for
0.5.10-*. Uh? Should be trivial to fix, especially considering that:

A Vcs-Git packaging repo is available in Debian, but apparently our
own one has no shared ancestor with it. I really don't get why. I see
the packaging was somehow inherited from there anyway, before Ubuntu
packaged 0.5.10, so it should be quite easy to get things right.
Reconciliating these histories is perhaps not a blocker for 1.1, *if*
someone commits to do it on their way to getting this package ready
for Debian. OTOH, *now* is probably the best time to rewrite the Git
history. Doing it later will be harder.

Please don't add "configure", or any such auto-generated autotools
files, to Git. That's a blocker.

The Homepage control field points to
https://launchpad.net/window-picker-applet, while you're packaging
a fork https://github.com/lanoxx/window-picker-applet.
That's a blocker.

I see the package build-depends on GTK2 stuff, while the GitHub fork
we're packaging says it's a port to GTK3. Upstream's README mentions
other build-deps, so it maybe something is wrong.

src/common.h is GPL-2+, and not mentionned in debian/copyright. Hint:
`licensecheck -r .' avoids to miss the most obvious problems of this
class. That's a blocker.

You know as well as I do why commit tickets and changelog entries like
"Update packaging" (4 files changed, 20 insertions(+), 104
deletions(-)) are to be avoided, so I won't state the obvious.
Too late for the commit message, but please fix debian/changelog.

And now, a few non-blockers (for 1.1).

The upstream tarball doesn't live in Git, so I had to go retrieve it
from the right suite in our APT repo. No big deal, but in the future
please prefer source format 3.0 (quilt) + git-buildpackage branches
layout, that is self-contained in Git, like we do for basically all of
our non-native packages (and that's also a very popular setup in
Debian as well). Consistency among packaging workflows makes it easier
for others to work on a package they've not touched before, makes the
RM's work easier, lowers the bar to contribution, and increases
chances we understand what we're doing when we type pseudo-random
lines in `$EDITOR debian/...'. Still, that's not a blocker, as long as
someone commits to do it while preparing the upload to Debian.

The obsolete debian/watch was dropped for some reason. It should be
updated instead, to point to the place where you're taking
releases from. Not a blocker, as long as [...].

You were concerned about the security of this code, so I suggest
building with all hardening flags, and not only the default ones, if
possible. Not a blocker, as long as [...].

I see that compat level was raised to 8. I'm curious why not use the
latest one, while you were at it?

Any particular reason why the Git repo isn't called neither the same
way as the source or binary package? If not, I'm happy to rename
it myself.

To end with, I've just documented in our Gitolite config's README how
to properly create a new repo. I've fixed the ones you created.

Cheers!