LGTM with a couple of minor changes. QA ok. Thank you!
https://codereview.appspot.com/132770043/diff/80001/quickstart/platform_support.py File quickstart/platform_support.py (right):
https://codereview.appspot.com/132770043/diff/80001/quickstart/platform_support.py#newcode30 quickstart/platform_support.py:30: utils Missing comma at the end of the line (utils,).
https://codereview.appspot.com/132770043/diff/80001/quickstart/platform_support.py#newcode134 quickstart/platform_support.py:134: logging.warn("a customized juju is being used.") no need for the final . here
https://codereview.appspot.com/132770043/diff/80001/quickstart/tests/test_platform_support.py File quickstart/tests/test_platform_support.py (right):
https://codereview.appspot.com/132770043/diff/80001/quickstart/tests/test_platform_support.py#newcode170 quickstart/tests/test_platform_support.py:170: class TestGetJujuCommand(unittest.TestCase): What do you think about changing from expected/actual to expected_command/assertTrue also in the first and third tests here? I read the second one way better.
https://codereview.appspot.com/132770043/diff/80001/quickstart/tests/test_platform_support.py#newcode178 quickstart/tests/test_platform_support.py:178: def test_getenv_fails_when_ignore_env_var(self): The test name here seems to no longer describe the goal.
https://codereview.appspot.com/132770043/
« Back to merge proposal
LGTM with a couple of minor changes.
QA ok.
Thank you!
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ platform_ support. py platform_ support. py (right):
File quickstart/
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ platform_ support. py#newcode30 platform_ support. py:30: utils
quickstart/
Missing comma at the end of the line (utils,).
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ platform_ support. py#newcode134 platform_ support. py:134: logging.warn("a customized juju is
quickstart/
being used.")
no need for the final . here
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ tests/test_ platform_ support. py tests/test_ platform_ support. py (right):
File quickstart/
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ tests/test_ platform_ support. py#newcode170 tests/test_ platform_ support. py:170: class and(unittest. TestCase) : command/ assertTrue also in the first and third tests here?
quickstart/
TestGetJujuComm
What do you think about changing from expected/actual to
expected_
I read the second one way better.
https:/ /codereview. appspot. com/132770043/ diff/80001/ quickstart/ tests/test_ platform_ support. py#newcode178 tests/test_ platform_ support. py:178: def fails_when_ ignore_ env_var( self):
quickstart/
test_getenv_
The test name here seems to no longer describe the goal.
https:/ /codereview. appspot. com/132770043/