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

Issue 13361043: WIP

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by rharding
Modified:
10 years, 8 months ago
Reviewers:
mp+182735, matthew.scott
Visibility:
Public.

Description

WIP https://code.launchpad.net/~rharding/juju-gui/renderBundle2/+merge/182735 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+498 lines, -12 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 chunk +1 line, -0 lines 0 comments Download
M app/assets/javascripts/d3-components.js View 3 chunks +3 lines, -0 lines 1 comment Download
M app/modules-debug.js View 1 chunk +3 lines, -0 lines 0 comments Download
A app/views/topology/bundle.js View 1 chunk +390 lines, -0 lines 4 comments Download
M app/views/topology/relation.js View 1 chunk +3 lines, -0 lines 0 comments Download
M app/views/topology/topology.js View 3 chunks +15 lines, -12 lines 0 comments Download
A test/data/wp-deployer.yaml View 1 chunk +41 lines, -0 lines 0 comments Download
M test/test_topology.js View 2 chunks +40 lines, -0 lines 2 comments Download

Messages

Total messages: 2
rharding
In general this looks good. Couple of nit-picks below. I'm not comfortable with so much ...
10 years, 8 months ago (2013-08-28 19:47:30 UTC) #1
matthew.scott
10 years, 8 months ago (2013-08-29 18:34:35 UTC) #2
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
Sign in to reply to this message.

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