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-comp... File app/assets/javascripts/d3-components.js (right): https://codereview.appspot.com/13361043/diff/1/app/assets/javascripts/d3-comp... app/assets/javascripts/d3-components.js:290: if (this.get('interactive') === 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 File app/views/topology/bundle.js (right): https://codereview.appspot.com/13361043/diff/1/app/views/topology/bundle.js#n... app/views/topology/bundle.js:80: //Process any changed data. space after the // https://codereview.appspot.com/13361043/diff/1/app/views/topology/bundle.js#n... app/views/topology/bundle.js:87: // enter enter? more or remove? https://codereview.appspot.com/13361043/diff/1/app/views/topology/bundle.js#n... app/views/topology/bundle.js:92: return (d.subordinate ? 'subordinate ' : '') + 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 File test/test_topology.js (right): https://codereview.appspot.com/13361043/diff/1/test/test_topology.js#newcode135 test/test_topology.js:135: // The size of the element shoul reflect teh passed in params the
LGTM, QA okay https://codereview.appspot.com/13361043/diff/1/app/views/topology/bundle.js File app/views/topology/bundle.js (right): https://codereview.appspot.com/13361043/diff/1/app/views/topology/bundle.js#n... app/views/topology/bundle.js:38: Manage service rendering and events. s/service/bundle https://codereview.appspot.com/13361043/diff/1/test/test_topology.js File test/test_topology.js (right): https://codereview.appspot.com/13361043/diff/1/test/test_topology.js#newcode135 test/test_topology.js:135: // The size of the element shoul reflect teh passed in params On 2013/08/28 19:47:30, rharding wrote: > the should