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

Issue 11804043: Fix subordinate behavior in Juju Core

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by gary.poster
Modified:
10 years, 9 months ago
Reviewers:
bac, benji, mp+176830
Visibility:
Public.

Description

Fix subordinate behavior in Juju Core The biggest change here is that we update service data with the charm subordinate flag, because juju core does not include that information in the service. This includes a test. Other changes here are fly-by. - The yui3-skin-sam class on the body was causing issues by adding styles unnecessarily. I put it on the subapp-browser, where it is sufficient. - The sandbox sends is_subordinate, not subordinate, for charms. I'm not sure if that is correct, but for now I changed charm.js to handle it. I'd welcome opinions on pushing this back down into the pyjuju sandbox, where I could test it. I'm kind of inclined to do that, actually, but I want to get this branch proposed. - I made a few changes to the inspector code to handle subordinates a bit better, particularly in the ghost inspector. the ghost inspector has no tests at all, and we have a card for adding them, so I did not try to tack that effort on to this branch. - I made a small tweak to charm-panel.js so that new services start looking like subordinates immediately. - I added a small hack that came in handy to track down behavior in a handlebars template. Just add {{debugger}} in the template and you will have a breakpoint there. handy. hopefully not too dangerous! We definitely don't want to check in a template that uses this, and the linter won't check our template code... https://code.launchpad.net/~gary/juju-gui/bug1204331/+merge/176830 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 5

Patch Set 2 : Fix subordinate behavior in Juju Core #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -11 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/index.html View 2 chunks +2 lines, -2 lines 0 comments Download
M app/models/charm.js View 1 1 chunk +4 lines, -1 line 0 comments Download
M app/store/endpoints.js View 1 chunk +2 lines, -0 lines 0 comments Download
M app/templates/ghost-config-wrapper.handlebars View 2 chunks +2 lines, -0 lines 0 comments Download
M app/views/charm-panel.js View 1 chunk +2 lines, -1 line 0 comments Download
M app/views/environment.js View 1 chunk +2 lines, -1 line 0 comments Download
M app/views/ghost-inspector.js View 2 chunks +7 lines, -3 lines 0 comments Download
M app/views/inspector.js View 1 chunk +6 lines, -3 lines 0 comments Download
M app/views/utils.js View 1 chunk +12 lines, -0 lines 0 comments Download
M test/test_endpoints.js View 1 chunk +25 lines, -0 lines 0 comments Download

Messages

Total messages: 4
gary.poster
Please take a look.
10 years, 9 months ago (2013-07-25 01:15:20 UTC) #1
bac
LGTM for code. QA shows the dismiss [X] on the inspector has dropped 50px or ...
10 years, 9 months ago (2013-07-25 12:34:29 UTC) #2
benji
The branch looks good. I just had a couple of small thoughts/praises/snarky comments. ;) LGTM-ly, ...
10 years, 9 months ago (2013-07-25 12:37:31 UTC) #3
gary.poster
10 years, 9 months ago (2013-07-25 15:20:32 UTC) #4
*** Submitted:

Fix subordinate behavior in Juju Core

The biggest change here is that we update service data with the charm
subordinate flag, because juju core does not include that information in the
service.  This includes a test.

Other changes here are fly-by.
- The yui3-skin-sam class on the body was causing issues by adding styles
unnecessarily.  I put it on the subapp-browser, where it is sufficient.
- The sandbox sends is_subordinate, not subordinate, for charms.  I'm not sure
if that is correct, but for now I changed charm.js to handle it.  I'd welcome
opinions on pushing this back down into the pyjuju sandbox, where I could test
it.  I'm kind of inclined to do that, actually, but I want to get this branch
proposed.
- I made a few changes to the inspector code to handle subordinates a bit
better, particularly in the ghost inspector.  the ghost inspector has no tests
at all, and we have a card for adding them, so I did not try to tack that effort
on to this branch.
- I made a small tweak to charm-panel.js so that new services start looking like
subordinates immediately.
- I added a small hack that came in handy to track down behavior in a handlebars
template.  Just add {{debugger}} in the template and you will have a breakpoint
there.  handy.  hopefully not too dangerous!  We definitely don't want to check
in a template that uses this, and the linter won't check our template code...

R=benji
CC=
https://codereview.appspot.com/11804043
Sign in to reply to this message.

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