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
signature.asc
Description: PGP signature
