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

Issue 10964044: Add expose/unexpose to the inspector.

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 1 month ago by bac
Modified:
6 years, 1 month ago
Reviewers:
jeff.pihach, benji, mp+173268
Visibility:
Public.

Description

Add expose/unexpose to the inspector. https://code.launchpad.net/~bac/juju-gui/inspector-expose/+merge/173268 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 29

Patch Set 2 : Add expose/unexpose to the inspector. #

Total comments: 3

Patch Set 3 : Add expose/unexpose to the inspector. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+224 lines, -14 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/templates/service-configuration.handlebars View 1 1 chunk +11 lines, -0 lines 0 comments Download
M app/views/environment.js View 1 3 chunks +4 lines, -5 lines 0 comments Download
M app/views/service.js View 1 6 chunks +56 lines, -9 lines 0 comments Download
M lib/views/juju-inspector.less View 1 chunk +26 lines, -0 lines 0 comments Download
M test/index.html View 1 1 chunk +1 line, -0 lines 0 comments Download
A test/test_inspector_settings.js View 1 2 1 chunk +124 lines, -0 lines 0 comments Download

Messages

Total messages: 6
bac
Please take a look.
6 years, 1 month ago (2013-07-05 19:22:20 UTC) #1
jeff.pihach
Looking good - I'd like to see some cleanup on the tests but thanks for ...
6 years, 1 month ago (2013-07-05 19:39:13 UTC) #2
benji
Looks good. I had a few small suggestions for your consideration. LGTM-ly y'rs, Benji https://codereview.appspot.com/10964044/diff/1/app/templates/service-configuration.handlebars ...
6 years, 1 month ago (2013-07-05 19:40:02 UTC) #3
bac
Please take a look. https://codereview.appspot.com/10964044/diff/1/app/templates/service-configuration.handlebars File app/templates/service-configuration.handlebars (right): https://codereview.appspot.com/10964044/diff/1/app/templates/service-configuration.handlebars#newcode9 app/templates/service-configuration.handlebars:9: <span></span> Probably. https://codereview.appspot.com/10964044/diff/1/app/templates/service-configuration.handlebars#newcode14 app/templates/service-configuration.handlebars:14: <hr ...
6 years, 1 month ago (2013-07-05 20:57:07 UTC) #4
jeff.pihach
LGTM great work, thanks for the updates. very small request in the tests before landing ...
6 years, 1 month ago (2013-07-05 21:03:46 UTC) #5
bac
6 years, 1 month ago (2013-07-05 21:21:49 UTC) #6
*** Submitted:

Add expose/unexpose to the inspector.

R=jeff.pihach, benji
CC=
https://codereview.appspot.com/10964044

https://codereview.appspot.com/10964044/diff/11001/test/test_inspector_settin...
File test/test_inspector_settings.js (right):

https://codereview.appspot.com/10964044/diff/11001/test/test_inspector_settin...
test/test_inspector_settings.js:77: env.destroy();
On 2013/07/05 21:03:46, jeff.pihach wrote:
> because destroy is async I would probably add a done() inside of a callback
> 
> env.after('destroy', function() { done(); });
> env.destroy();

Done.
Sign in to reply to this message.

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