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

Proposed by Gary Poster
Status: Merged
Merged at revision: 1179
Proposed branch: lp://staging/~gary/juju-gui/bug1247903
Merge into: lp://staging/juju-gui/experimental
Diff against target: 394 lines (+239/-39)
5 files modified
app/store/env/simulator.js (+28/-30)
app/templates/unit-action-buttons.handlebars (+1/-1)
app/views/viewlets/service-overview.js (+9/-5)
test/test_inspector_overview.js (+102/-0)
test/test_simulator.js (+99/-3)
To merge this branch: bzr merge lp://staging/~gary/juju-gui/bug1247903
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+193875@code.staging.launchpad.net

Description of the change

Re-enable Landscape sim and fix inspector link

Changes to move units to service attributes broke the Landscape simulation because they were buggy, and we had no tests. Fixing that revealed that Landscape links were broken in the inspector if units appeared with Landscape problems while you observed the inspector. This branch also fixes that problem.

To QA, run the simulator on the sandbox and create 100 or 200 units in a service. Keep the inspector open after creation. You should see a few Landscape issues appear within a few seconds. If you open up the Landscape sections of the inspector, the link to Landscape at the bottom of the section should have a href that looks vaguely like it might point to Landscape, if Landscape were really hooked up.

https://codereview.appspot.com/21440044/

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

Reviewers: mp+193875_code.launchpad.net,

Message:
Please take a look.

Description:
Re-enable Landscape sim and fix inspector link

Changes to move units to service attributes broke the Landscape
simulation because they were buggy, and we had no tests. Fixing that
revealed that Landscape links were broken in the inspector if units
appeared with Landscape problems while you observed the inspector. This
branch also fixes that problem.

To QA, run the simulator on the sandbox and create 100 or 200 units in a
service. Keep the inspector open after creation. You should see a few
Landscape issues appear within a few seconds. If you open up the
Landscape sections of the inspector, the link to Landscape at the bottom
of the section should have a href that looks vaguely like it might point
to Landscape, if Landscape were really hooked up.

Thank you.

https://code.launchpad.net/~gary/juju-gui/bug1247903/+merge/193875

(do not edit description out of merge proposal)

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

Affected files (+241, -39 lines):
   A [revision details]
   M app/store/env/simulator.js
   M app/templates/unit-action-buttons.handlebars
   M app/views/viewlets/service-overview.js
   M test/test_inspector_overview.js
   M test/test_simulator.js

Revision history for this message
Richard Harding (rharding) wrote :

LGTM thanks for the update and tests.

https://codereview.appspot.com/21440044/

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

*** Submitted:

Re-enable Landscape sim and fix inspector link

Changes to move units to service attributes broke the Landscape
simulation because they were buggy, and we had no tests. Fixing that
revealed that Landscape links were broken in the inspector if units
appeared with Landscape problems while you observed the inspector. This
branch also fixes that problem.

To QA, run the simulator on the sandbox and create 100 or 200 units in a
service. Keep the inspector open after creation. You should see a few
Landscape issues appear within a few seconds. If you open up the
Landscape sections of the inspector, the link to Landscape at the bottom
of the section should have a href that looks vaguely like it might point
to Landscape, if Landscape were really hooked up.

R=rharding
CC=
https://codereview.appspot.com/21440044

https://codereview.appspot.com/21440044/

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

On 2013/11/05 12:18:05, rharding wrote:
> LGTM thanks for the update and tests.

Thank you for the review, Rick.

https://codereview.appspot.com/21440044/

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