[Piuparts-devel] Bug#893731: Bug#893731: [piuparts] Please add docker support

[Holger Levsen]

On Sat, Mar 24, 2018 at 01:06:43PM -0300, Agustin Henze wrote:
> >> -  Use schroot session named SCHROOT-NAME for the chroot, instead of building a new one with debootstrap.
> >> +  Use schroot session named SCHROOT-NAME for the testing environment.
> > why drop the second half here?
> Not really sure, maybe I thought it was more clear. But now you mention it, I
> prefer to keep it hehe. I've re-added it
 
:)

[...]
> done
[...]
> Done
[...]
> Done

thanks!

> >> --- a/piuparts.py
> >> +++ b/piuparts.py
> > [...]
> >> +import json
> > doesn't this need a new dependency?
> Noup, it's part of the stdlib
> libpython3.6-stdlib:amd64: /usr/lib/python3.6/json/__init__.py
> libpython2.7-stdlib:amd64: /usr/lib/python2.7/json/__init__.py

thanks for explaining & confirming!

> >>      def create_resolv_conf(self):
> >> +        if settings.docker_image:
> >> +            # Do nothing, docker already takes care of this
> >> +            return
> > 
> > and
> > 
> >>      def terminate_running_processes(self):
> >> +        if settings.docker_image:
> >> +            # docker takes care of this
> >> +            return
> > 
> > here I wonder: is it really cleaner to return if docker is used or
> > wouldnt it be better to not call it in the first place?
> 
> Maybe, I'm not totally sure, but if you feel it's better, I prefer the way you
> feel more comfortable :). After all, it's your little creature hehe
 
I've now looked at the surrounding code and indeed I would like these
checks to be moved to the calling functions. check_for_no_processes()
and configure_chroot() already have other conditional code blocks moving
the code from "above" there seems better indeed.

> > The rest looks fine to me and I'll happily merge once you fixed up those
> > little things. Thanks again!
> Great! Please take a look at it again. Also I've made little fixes that Andreas
> Beckman and Iñaki Malerva point me on the PR ¯\_(ツ)_/¯. Basically add docker.io
> as suggested package, edit debian/changelog and fix a typo in doc.

thanks!

I guess we are really close to merging this now! \o/


-- 
cheers,
	Holger
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/piuparts-devel/attachments/20180325/772ba84a/attachment.sig>