Code review comment for lp:~bcsaller/juju-gui/viewmodel-improvements

I was in the middle of a review--with a long hiatus for various calls
and other responsibilities--when you made the new version. These
comments are from the previous revision, so they may or may not still be
pertinent. I'm sending them off and will start a new review after
lunch.

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

https://codereview.appspot.com/7231067/diff/1/app/views/topology/service.js#newcode102
app/views/topology/service.js:102: serviceDblClick: function(d, self) {
On 2013/01/31 10:44:24, teknico wrote:
> A more descriptive name than "d" on these methods would be nice, some
day.

+1

https://codereview.appspot.com/7231067/diff/1/app/views/topology/service.js#newcode201
app/views/topology/service.js:201: var service = box;
Shouldn't this be box.model? If so, why does it still work? If it is
really ok for this to be the box, why should we call it the service?

Please address this one way or another so it is clear to the reader what
is going on--and, if necessary, tested that the method is doing what we
expect.

https://codereview.appspot.com/7231067/diff/1/app/views/topology/service.js#newcode242
app/views/topology/service.js:242: views.toBoundingBoxes(this,
db.services, this.service_boxes);
So, this mutates something or other? I need to go read it...

https://codereview.appspot.com/7231067/diff/1/app/views/topology/service.js#newcode243
app/views/topology/service.js:243: this.services =
Y.Object.values(this.service_boxes);
Why do we have to stash this?

If we really want this on the module, I'd like it initialized and
explained in the initializer, the way you do with service_boxes.

https://codereview.appspot.com/7231067/diff/1/app/views/topology/service.js#newcode245
app/views/topology/service.js:245: // XXX: containment breaking alias,
do we need this?
If we do, then we should only store service_boxes on topo, and not
locally on "this," right? (No need to change this now, just a fly-by
thought.)

https://codereview.appspot.com/7231067/