Merge lp://staging/~rharding/juju-gui/renderBundle2 into lp://staging/juju-gui/experimental
Proposed by
Richard Harding
Status: | Work in progress |
---|---|
Proposed branch: | lp://staging/~rharding/juju-gui/renderBundle2 |
Merge into: | lp://staging/juju-gui/experimental |
Diff against target: |
623 lines (+496/-12) 8 files modified
app/app.js (+1/-0) app/assets/javascripts/d3-components.js (+3/-0) app/modules-debug.js (+3/-0) app/views/topology/bundle.js (+390/-0) app/views/topology/relation.js (+3/-0) app/views/topology/topology.js (+15/-12) test/data/wp-deployer.yaml (+41/-0) test/test_topology.js (+40/-0) |
To merge this branch: | bzr merge lp://staging/~rharding/juju-gui/renderBundle2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju GUI Hackers | Pending | ||
Review via email:
|
Description of the change
To post a comment you must log in.
Unmerged revisions
- 988. By Richard Harding
-
Sync
Reviewers: mp+182735_ code.launchpad. net,
Message:
In general this looks good. Couple of nit-picks below. I'm not
comfortable with so much code dupe between bundle.js and service.js
though so not ok'ing all the way.
https:/ /codereview. appspot. com/13361043/ diff/1/ app/assets/ javascripts/ d3-components. js javascripts/ d3-components. js (right):
File app/assets/
https:/ /codereview. appspot. com/13361043/ diff/1/ app/assets/ javascripts/ d3-components. js#newcode290 javascripts/ d3-components. js:290: if (this.get( 'interactive' )
app/assets/
=== false) { return; }
what is the interactive attr? I don't see it defined and not following
what this is doing.
https:/ /codereview. appspot. com/13361043/ diff/1/ app/views/ topology/ bundle. js topology/ bundle. js (right):
File app/views/
https:/ /codereview. appspot. com/13361043/ diff/1/ app/views/ topology/ bundle. js#newcode80 topology/ bundle. js:80: //Process any changed data.
app/views/
space after the //
https:/ /codereview. appspot. com/13361043/ diff/1/ app/views/ topology/ bundle. js#newcode87 topology/ bundle. js:87: // enter
app/views/
enter? more or remove?
https:/ /codereview. appspot. com/13361043/ diff/1/ app/views/ topology/ bundle. js#newcode92 topology/ bundle. js:92: return (d.subordinate ? 'subordinate '
app/views/
: '') +
so is this repeated? Can it be shared? A typology.utils helper or
something?
https:/ /codereview. appspot. com/13361043/ diff/1/ test/test_ topology. js topology. js (right):
File test/test_
https:/ /codereview. appspot. com/13361043/ diff/1/ test/test_ topology. js#newcode135 topology. js:135: // The size of the element shoul reflect teh
test/test_
passed in params
the
Description:
WIP
https:/ /code.launchpad .net/~rharding/ juju-gui/ renderBundle2/ +merge/ 182735
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/13361043/
Affected files: javascripts/ d3-components. js debug.js topology/ bundle. js topology/ relation. js topology/ topology. js wp-deployer. yaml topology. js
A [revision details]
M app/app.js
M app/assets/
M app/modules-
A app/views/
M app/views/
M app/views/
A test/data/
M test/test_