Code review comment for lp://qastaging/~frankban/juju-quickstart/maas-detect

Revision history for this message
Francesco Banconi (frankban) wrote :

Please take a look.

https://codereview.appspot.com/157830043/diff/1/quickstart/maas.py
File quickstart/maas.py (right):

https://codereview.appspot.com/157830043/diff/1/quickstart/maas.py#newcode42
quickstart/maas.py:42: Return None if if no authenticated API is found.
On 2014/10/10 08:48:53, bac wrote:
> s/if if/if/

Done.

https://codereview.appspot.com/157830043/diff/1/quickstart/models/envs.py
File quickstart/models/envs.py (right):

https://codereview.appspot.com/157830043/diff/1/quickstart/models/envs.py#newcode388
quickstart/models/envs.py:388: # Assume all missing fields can be
automatically generated.
On 2014/10/10 08:48:53, bac wrote:
> What happens if this assumption is wrong? Will field.generate() raise
an error?

A field without the ability to automatically generate a value does not
have the generate method. So the above call would raise an
AttributeError.

https://codereview.appspot.com/157830043/diff/1/quickstart/tests/cli/test_views.py
File quickstart/tests/cli/test_views.py (right):

https://codereview.appspot.com/157830043/diff/1/quickstart/tests/cli/test_views.py#newcode64
quickstart/tests/cli/test_views.py:64: default values: "maas-user",
"http://1.2.3.4/MAAS" and "maas-secret".
On 2014/10/10 08:48:53, bac wrote:
> Maybe not repeat the values set above but just reference them? I
reckon DRY
> should apply to comments too.

Done.

https://codereview.appspot.com/157830043/

« Back to merge proposal