Merge lp://staging/~gary/juju-gui/sandboxEnv-4 into lp://staging/juju-gui/experimental

Reviewers: mp+154827_code.launchpad.net,

Message:
Please take a look.

Description:
Connect sandbox environment to app

I apologize for this branch's size. On one of my commits I decided to
change the docstring approach for my two files into the YUI approach,
because I liked it so much better. I didn't realize how big the changes
were.

Beyond that, a few other 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.

To QA, in config-debug.js add a "sandbox: true" flag, start up make
devel, and deploy some things. You can't do anything other than deploy
at the moment. :-)

https://code.launchpad.net/~gary/juju-gui/sandboxEnv-4/+merge/154827

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7896045/

Affected files:
   A [revision details]
   M app/app.js
   M app/index.html
   M app/models/models.js
   M app/store/env/fakebackend.js
   M app/store/env/sandbox.js
   M app/views/charm-panel.js
   M test/test_app.js
   M test/test_fakebackend.js
   M test/test_sandbox.js
   M test/utils.js

Thanks for this! Mostly trivial comments except for the one in
test_app.js which we are chatting about on IRC.

https://codereview.appspot.com/7896045/diff/1/app/index.html
File app/index.html (right):

https://codereview.appspot.com/7896045/diff/1/app/index.html#newcode237
app/index.html:237: setLoadingMessageText('Connecting to the Juju
environment');
Yay! Thinking positive! :-)

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
I think that this comment needs a comment defining what `verisimilitude`
means

;-)

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
possible typo? change_type to be changeType

https://codereview.appspot.com/7896045/diff/1/app/store/env/sandbox.js#newcode327
app/store/env/sandbox.js:327: performOp_deploy: function(data) {
Instead of using _ on these methods to match the previous fn maybe we
should change the old one to camelCase

https://codereview.appspot.com/7896045/diff/1/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/7896045/diff/1/test/test_app.js#newcode408
test/test_app.js:408: app = new Y.juju.App(
The app now requires that an element is in the DOM at the time of
instantiation (that's in the index.html by default) so this test will
fail once being merged with trunk.

The cause:
http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/view/head:/app/app.js#L389

The fix:
http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/view/head:/test/test_notifications.js#L76

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

LGTM with the test fix (better sooner, I think) - thanks for the branch!

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

LGTM - thanks for making that fix!

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

*** 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/

Thank you both for the reviews!

Gary

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