Code review comment for lp://staging/~gary/juju-gui/sandboxEnv-2

Revision history for this message
Brad Crittenden (bac) wrote :

LGTM modulo a couple of changes.

Before we'd not done any YAML parsing in JS due to Kapil's assertion
that there were no libraries that were acceptable. Is the one you've
chosen suitable? Can it be used in the parsing of config files when the
user uploads one? If so, that would make passing YAML obsolete and
would give a better user experience.

https://codereview.appspot.com/7741043/diff/1/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/7741043/diff/1/app/store/env/fakebackend.js#newcode272
app/store/env/fakebackend.js:272: }
Originally improv accepted both and just gave precedence to configYAML.
The go implementation does the same so it would be nice for this
implementation to be consistent.

https://codereview.appspot.com/7741043/diff/1/test/test_fakebackend.js
File test/test_fakebackend.js (right):

https://codereview.appspot.com/7741043/diff/1/test/test_fakebackend.js#newcode200
test/test_fakebackend.js:200:
This test will need to be fixed if you make the previously suggested
change.

https://codereview.appspot.com/7741043/

« Back to merge proposal