https://codereview.appspot.com/7132061/diff/6001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/7132061/diff/6001/app/app.js#newcode351
app/app.js:351: active.update();
On 2013/01/20 15:58:45, gary.poster wrote:
> cool

Thanks, cutting to this should actually be a big improvement. I was
worried there would still be more work left before this would work
properly but it seems fine like this. This cuts the number of render
passes substantially and reduces the amount of work we do quite a lot.

https://codereview.appspot.com/7132061/diff/6001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7132061/diff/6001/app/views/topology/service.js#newcode33
app/views/topology/service.js:33: * relation.
On 2013/01/20 15:58:45, gary.poster wrote:
> We now have this comment both in the event registration and in the
function
> definition.  I'm not sure that inline event handler functions don't
have their
> place, to be honest.  If we do separate them, I think we can only
expect to
> maintain comments in one place.  I believe that the place should be
the
> function, so I would recommend deleting this copy of the comment.

Agreed with comment placement, this was the result of trying to quickly
cut a smaller branch, bad porting.

https://codereview.appspot.com/7132061/diff/6001/app/views/topology/service.js#newcode65
app/views/topology/service.js:65: toggleControlPanel: {callback:
function() {
On 2013/01/20 15:58:45, gary.poster wrote:
> I prefer the change that Nicola made, that you have reverted.  In that
change,
> he renamed "ControlPanel" to "ServiceMenu," which seemed like a naming
> improvement to Nicola and to his two reviewers.  He also was able to
be explicit
> that we only needed to "hide" here, which would be nice if we can
manage it.

Not intentional, this is how the trunk merge fell out. I'll restore his
version (which I had a hand in as well and agreed with).

https://codereview.appspot.com/7132061/diff/6001/app/views/topology/service.js#newcode178
app/views/topology/service.js:178:
self.service_click_actions.toggleControlPanel(null, self);
On 2013/01/20 15:58:45, gary.poster wrote:
> You've reverted Nicola's changes.  I think they were improvements, but
even if
> you disagree, socially a discussion would be better.  Please don't do
this.

Bad Merge, fixed.

https://codereview.appspot.com/7132061/diff/6001/app/views/topology/service.js#newcode201
app/views/topology/service.js:201: .toggleControlPanel(box, context);
On 2013/01/20 15:58:45, gary.poster wrote:
> You've reverted Nicola's changes.  Please don't do this.

Same

https://codereview.appspot.com/7132061/diff/6001/test/test_environment_view.js
File test/test_environment_view.js (right):

https://codereview.appspot.com/7132061/diff/6001/test/test_environment_view.js#newcode112
test/test_environment_view.js:112: db.on_delta({data:
Y.clone(environment_delta)});
This change may be worth of review comment. This is needed because we
delete annotations once their x/y has been read and applied. This
creates an issue in testing because its a reference all the way back to
the original test data stub.

https://codereview.appspot.com/7132061/diff/6001/undocumented
File undocumented (left):

https://codereview.appspot.com/7132061/diff/6001/undocumented#oldcode1
undocumented:1: app/app.js:599 "callback"
On 2013/01/20 15:58:45, gary.poster wrote:
> You increased the undocumented count from 241 to 246.  It would be
nice if we at
> least kept the number stable across these refactorings.  Given my
desire to be
> able to declare this complete by Monday, or Tuesday at the latest, I
won't stop
> you from landing for this, but maybe you could come back to write up
five
> methods later.

Added some doc strings, I think documenting callback handlers with a
full doc strings when they have uniform arguments and appeal to a
documented usage pattern is a bit of an anti-pattern (not that this is
the only type of doc I haven't written ;) . Happy to talk about this in
a Friday call if its an issue.

https://codereview.appspot.com/7132061/