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

Proposed by Gary Poster
Status: Merged
Merged at revision: 565
Proposed branch: lp://staging/~gary/juju-gui/bug1169350
Merge into: lp://staging/juju-gui/experimental
Diff against target: 264 lines (+53/-63)
6 files modified
app/store/env/fakebackend.js (+2/-0)
app/views/topology/relation.js (+1/-1)
test/test_app.js (+1/-2)
test/test_fakebackend.js (+18/-20)
test/test_sandbox.js (+1/-1)
test/utils.js (+30/-39)
To merge this branch: bzr merge lp://staging/~gary/juju-gui/bug1169350
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+159271@code.staging.launchpad.net

Description of the change

gallimaufry

Title is in homage to benji's potpourri.

- Fix 1169350: sandbox was not marking subordinate charms correctly.
- Fix 1169668: subordinate relations were not shown because of the id change.
- Simplify test charm store code.

https://codereview.appspot.com/8812043/

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :

Reviewers: mp+159271_code.launchpad.net,

Message:
Please take a look.

Description:
gallimaufry

Title is in homage to benji's potpourri.

- Fix 1169350: sandbox was not marking subordinate charms correctly.
- Fix 1169668: subordinate relations were not shown because of the id
change.
- Simplify test charm store code.

https://code.launchpad.net/~gary/juju-gui/bug1169350/+merge/159271

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/store/env/fakebackend.js
   M app/views/topology/relation.js
   M test/test_app.js
   M test/test_fakebackend.js
   M test/test_sandbox.js
   M test/utils.js

Revision history for this message
Madison Scott-Clary (makyo) wrote :

LGTM - thanks for the fixes and the test util.

https://codereview.appspot.com/8812043/

Revision history for this message
Jeff Pihach (hatch) wrote :

LGTM nice improvements!

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

https://codereview.appspot.com/8812043/diff/1/app/store/env/fakebackend.js#newcode344
app/store/env/fakebackend.js:344: data.is_subordinate =
data.subordinate;
Ahh it was a naming mixup.

https://codereview.appspot.com/8812043/diff/1/test/utils.js
File test/utils.js (right):

https://codereview.appspot.com/8812043/diff/1/test/utils.js#newcode62
test/utils.js:62: _cached_charms: (function() {
Great idea

https://codereview.appspot.com/8812043/

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

*** Submitted:

gallimaufry

Title is in homage to benji's potpourri.

- Fix 1169350: sandbox was not marking subordinate charms correctly.
- Fix 1169668: subordinate relations were not shown because of the id
change.
- Simplify test charm store code.

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

https://codereview.appspot.com/8812043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches