Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(868)

Issue 10522043: Add add_units, expose, & unexpose to Go Sandbox.

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 9 months ago by bac
Modified:
6 years, 9 months ago
Reviewers:
mp+171155, frankban, jeff.pihach, teknico
Visibility:
Public.

Description

Add add_units, expose, & unexpose to Go Sandbox. https://code.launchpad.net/~bac/juju-gui/go-sandbox-addunits-expose-unexpose/+merge/171155 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 13

Patch Set 2 : Add add_units, expose, & unexpose to Go Sandbox. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+492 lines, -7 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/store/env/fakebackend.js View 1 2 chunks +4 lines, -4 lines 0 comments Download
M app/store/env/sandbox.js View 1 2 chunks +58 lines, -1 line 0 comments Download
M test/test_fakebackend.js View 1 1 chunk +117 lines, -0 lines 0 comments Download
M test/test_sandbox.js View 1 3 chunks +311 lines, -2 lines 0 comments Download

Messages

Total messages: 5
bac
Please take a look.
6 years, 9 months ago (2013-06-24 19:29:16 UTC) #1
jeff.pihach
LGTM - Thanks for this. just a few trivials below. https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js File app/store/env/sandbox.js (right): https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js#newcode767 ...
6 years, 9 months ago (2013-06-24 19:56:08 UTC) #2
frankban
LGTM, thank you.
6 years, 9 months ago (2013-06-25 09:45:11 UTC) #3
teknico
LGTM. Again, thanks for adapting those support functions from the PyJuju sandbox test suite. See ...
6 years, 9 months ago (2013-06-25 11:09:40 UTC) #4
bac
6 years, 9 months ago (2013-06-25 13:06:05 UTC) #5
*** Submitted:

Add add_units, expose, & unexpose to Go Sandbox.

R=jeff.pihach, frankban, teknico
CC=
https://codereview.appspot.com/10522043

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

https://codereview.appspot.com/10522043/diff/1/app/store/env/sandbox.js#newco...
app/store/env/sandbox.js:767: //client.receiveNow(data);
On 2013/06/24 19:56:08, jeff.pihach wrote:
> Remove

Done.

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

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:331: fakebackend.logout();
Boo!  Hiss!

https://codereview.appspot.com/10522043/diff/1/test/test_fakebackend.js#newco...
test/test_fakebackend.js:347: '`Je ne suis pas un service` is an invalid service
name.',
The test is merely testing what the fakebackend was issuing as an error message,
so the backquotes here were required.

It is sufficiently odd that I changed fakebackend.js to use single quotes
instead and updated these tests.

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js
File test/test_sandbox.js (right):

https://codereview.appspot.com/10522043/diff/1/test/test_sandbox.js#newcode1436
test/test_sandbox.js:1436: 
On 2013/06/24 19:56:08, jeff.pihach wrote:
> remove :)

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b