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

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Connect sandbox environment to app

Beyond some doc formatting changes, a few fly-by spelling corrections,
some changes I needed for the tests to pass, and so on, this branch is
about finally connecting the in-browser sandbox environment work with
the app. I added the delta stream and made it possible to use the
sandbox environment by using configuration value.

R=jeff.pihach, matthew.scott
CC=
https://codereview.appspot.com/7896045

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

https://codereview.appspot.com/7896045/diff/1/app/store/env/sandbox.js#newcode244
app/store/env/sandbox.js:244: // For fuller verisimilitude, we could
convert some of the
On 2013/03/21 23:10:15, jeff.pihach wrote:
> I think that this comment needs a comment defining what
`verisimilitude` means

> ;-)
sorry, my parents had multiple English degrees so they actually used
this stuff growing up. :-)

https://codereview.appspot.com/7896045/diff/1/app/store/env/sandbox.js#newcode248
app/store/env/sandbox.js:248: // The unit change_type is actually
"serviceUnit" in the Python
On 2013/03/21 23:10:15, jeff.pihach wrote:
> possible typo? change_type to be changeType

Done.

https://codereview.appspot.com/7896045/diff/1/app/store/env/sandbox.js#newcode327
app/store/env/sandbox.js:327: performOp_deploy: function(data) {
On 2013/03/21 23:10:15, jeff.pihach wrote:
> Instead of using _ on these methods to match the previous fn maybe we
should
> change the old one to camelCase

Too big of a change for this branch. We need an underline-geddon if
we're going to do that.

https://codereview.appspot.com/7896045/

« Back to merge proposal