Very nice first Python branch Jeff, thank you! Some changes/comments follow. Please let me know what do you think about the strategy I propose below. 9 +SYSDEPS = build-essential python-pip python-virtualenv python-dev Nice! Could you please keep those dependencies in alphabetical order? 27 + raise ProgramExit('Unable to retrieve juju version: {}'.format(err)) 28 + raise ProgramExit('unable to retrieve Juju version: {}'.format(err)) Also the empty line can be removed. 42 + """Easy quickstart.utils.get_juju_version stub""" Please use a full sentence, and complete the sentence with a dot. Also, I am not sure this is strictly required, see below. I am considering a change in how app.bootstrap works. It could just take a requires_sudo argument: this way it can be nicely tested and the arguments are more understandable. Basically, I'd like something like the following in manage: requires_sudo = False if is_local: version = ... requires_sudo = utils.local_bootstrap_requires_sudo(version) already_bootstrapped, bsn_series = app.bootstrap( options.env_name, requires_sudo=requires_sudo, debug=options.debug) This way we also call "juju version" only when required. 64 + helpers.CallTestsMixin, ProgramExitTestsMixin, unittest.TestCase, 65 + helpers.GetJujuVersionMixin): Please move the GetJujuVersionMixin after CallTestsMixin. Anyway, if you decide to make the change I suggested above, this should be no longer required. 186 + # Should return a numeric string from the juju version Please complete the sentence. 192 + # If Juju version returns an error this will throw Ditto. 202 + # If Juju version is lower than 1.17.1 Ditto. 204 + self.assertEqual(True, value) This can also be written as: self.assertTrue(value). 207 + # If juju version is higher than 1.16.X Please complete the sentence. 209 + self.assertEqual(False, value) Same as above (assertFalse). 212 + # If deploying to a cloud env 214 + self.assertEqual(False, value) ... 223 +def get_juju_version(): What do you think about making this function return a tuple like the following: (major:int, minor:int, patch:str)? This way, future code which needs to perform version checks can just call this function avoiding further parsing. Something like: def get_juju_version(): """Return the current juju-core version. Return a (major:int, minor:int, patch:bytes) tuple, including major, minor and patch version numbers. Raise a ValueError if the "juju version" call exits with an error or the returned version is not well formed. """ retcode, output, error = call('juju', 'version') if retcode: raise ValueError(error) version_string = output.split('-')[0] try: major, minor, patch = version_string.split('.', 2) return int(major), int(minor), patch except ValueError: msg = 'invalid version string: {}'.format(version_string) raise ValueError(msg.encode('utf-8')) What do you think? 234 +def bootstrap_requires_sudo(is_local, version): 235 + """After Juju version 1.17.0 sudo is no longer required for 236 + bootstrapping local deployments. 237 + """ At this point this could be called local_bootstrap_requires_sudo and just receive the version tuple. Also, when writing docstrings, please keep the first sentence in a single line.