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.
Affected files:
A [revision details]
M app/app.js
M app/assets/javascripts/d3-components.js
M app/modules-debug.js
A app/views/topology/bundle.js
M app/views/topology/relation.js
M app/views/topology/topology.js
A test/data/wp-deployer.yaml
M test/test_topology.js
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_