[pkg-go] Call for review - containerd and dependencies

[Michael Stapelberg]

On Wed, Feb 28, 2018 at 3:49 AM, Arnaud <arnaud.rebillout at collabora.com>
wrote:

>
>
> On 02/27/2018 03:32 AM, Michael Stapelberg wrote:
> > Once you feel your packages are ready for review (possibly this reply
> > prompts some changes?), please send an up-to-date list of git URLs to
> > what you’d like reviewed. Thanks!
>
> Ok, here is the package list again, I prefer to leave all the packages
> on my personal salsa space for now, just so that I can fix every details
> according to your feedback, then cleanup the git history, and then move
> that definitive version to alioth or salsa, where applicable.
>
> URLS are the same as the initial email, it's just a copy/paste of the
> initial email, from which I removed comments that are not applicable
> anymore.
>
>
>
> BUMPED PACKAGES
> ---------------
>
> #### containerd
> <https://salsa.debian.org/elboulangero-guest/containerd>
> - As for testing the thing really, I didn't go far yet, I just launched
>   the binary, the log messages that appeared looked healthy enough,
>   and that's all for now.
>

Some of the comments below apply (unnecessary-team-upload, DEP-3 headers
for patches).

Also, this requires golang-github-containerd-console-dev to build, which is
not in the archive and not in this list of packages?


>
> #### golang-github-docker-go-events
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-docker-go-events>
>

Can you address these please?
P: golang-github-docker-go-events source: file-contains-trailing-whitespace
debian/control (line 25)
I: golang-github-docker-go-events source: vcs-field-uses-insecure-uri
vcs-git git://
anonscm.debian.org/pkg-go/packages/golang-github-docker-go-events.git
W: golang-github-docker-go-events source: unnecessary-team-upload

Aside from that, it looks good.


>
> #### golang-github-docker-go-metrics
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-docker-go-metrics>
>

Can’t build this until golang-github-prometheus-client-golang is fixed.
Some of the lintian warnings above seem to apply here, too, though.


>
> #### golang-github-opencontainers-image-spec
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-opencontainers-image-spec>
> - I noticed only after doing all the work that Michael Stapelberg was
>   working on this package lately. I hope I don't duplicate the work.
> - I'm not sure about the gbp.conf commit, please confirm whether
>   it's ok.
>

The gbp.conf change is okay. We should eventually clean up gbp.conf across
our packages, but we have enough going on for now.

The package looks good.


>
> #### golang-github-prometheus-client-golang
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-prometheus-client-golang>
>

This package cannot be cloned with gbp as-is:

% gbp clone
https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang
gbp:info: Cloning from '
https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang
'
gbp:error: Git command failed: Error running git checkout: error: pathspec
'master' did not match any file(s) known to git.

I think you’ll need to set debian-branch in debian/gbp.conf.


>
>
>
> NEW PACKAGES
> ------------
>
> #### continuity
> <https://salsa.debian.org/elboulangero-guest/continuity>
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890983>
> - I ran the series of test described at
>   <https://github.com/containerd/continuity>,
>   everything went fine.
>

It would be good to add a manpage for the continuity binary. Otherwise, the
package looks fine.


>
> #### golang-gogottrpc
> <https://salsa.debian.org/elboulangero-guest/golang-gogottrpc>
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890958>
> - Patch and issues submitted upstream.
>

Can you add DEP-3 tags to the patches please? See
http://dep.debian.net/deps/dep3/. No need to go overboard, just the
last-updated/author/description/forwarded ones.

Also, there seems to be a lintian false-positive: it reports
statically-linked-binary despite the Build-Depends on golang-go. Can you
file a lintian bug about that please?


>
> #### golang-github-containerd-btrfs
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-containerd-btrfs>
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890989>
> - I disabled the test to avoid installing a btrfs-test binary. I don't
>   think it makes sense to create an additional binary package
>   just for shipping this test.
>

Sounds good. Maybe change cmd to cmd/btrfs-test so that we’ll notice newly
introduced binaries upon updates? Otherwise, the package looks good.


>
> #### golang-github-containerd-typeurl
> <https://salsa.debian.org/elboulangero-guest/golang-
> github-containerd-typeurl>
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891181>
> - Patch submitted upstream
>

Please add DEP-3 tags to the patch. Looks good otherwise.


>
> #### golang-github-dmcgowan-go-tar
> <https://salsa.debian.org/elboulangero-guest/golang-github-dmcgowan-go-tar
> >
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890960>
> - This one was a tricky one, please read the commit messages for details.
>

Does the package which needs this require support for sparse tar files? If
not, I think we can do away with this package in favor of patching the
consumer to use archive/tar instead. Doing a diff -ur shows that this
package is identical to archive/tar as of Go 1.10, except for the sparse
support, which is scheduled to land in Go 1.11.


>
>
> Best,
>
>   Arnaud
>



-- 
Best regards,
Michael
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pkg-go-maintainers/attachments/20180228/908c3235/attachment.html>