Code review comment for lp:~frankban/juju-quickstart/more-cloud-providers

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

Please take a look.

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

https://codereview.appspot.com/52080044/diff/1/quickstart/models/envs.py#newcode529
quickstart/models/envs.py:529: fields.ChoiceField(
On 2014/01/15 19:29:04, rharding wrote:
> should this be a suggestion field to help users move along faster?
Just suggest
> a sensible default like US East which I think even things like the AMZ
web ui
> does? It's a suggestion field in the open stack example above.

Suggestion fields are used to suggest a value while leaving the field
open to arbitrary values. The choice field here means validation is
involved. AFAICT Azure does not use US East as default.

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

https://codereview.appspot.com/52080044/diff/1/quickstart/tests/cli/test_forms.py#newcode174
quickstart/tests/cli/test_forms.py:174: class
TestCreateStringWidget(ChoicesTestsMixin, unittest.TestCase):
On 2014/01/15 19:29:04, rharding wrote:
> I think that the suggestion field is its own new class and should have
its own
> tests vs getting shoed into the String widget tests. This seems like a
path down
> like the one in the charm where the one suite of tests contained
others that
> weren't directly related.

I am not sure I understood this comment. The suggestion field itself
already has its own tests in test_fields.py. Here we are just testing
that suggestions are included in the string widget if required.

https://codereview.appspot.com/52080044/

« Back to merge proposal