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

Revision history for this message
Benjamin Saller (bcsaller) wrote :

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#newcode243
app/views/topology/service.js:243: this.services =
Y.Object.values(this.service_boxes);
On 2013/01/31 17:30:12, gary.poster wrote:
> 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.

removed.

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?
On 2013/01/31 17:30:12, gary.poster wrote:
> 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.)
Moved to topo.

https://codereview.appspot.com/7231067/diff/10/app/views/utils.js
File app/views/utils.js (right):

https://codereview.appspot.com/7231067/diff/10/app/views/utils.js#newcode790
app/views/utils.js:790: getNearestConnector: {
On 2013/02/01 02:26:05, gary.poster wrote:
> Now that it is a property, wouldn't this be better as a noun,
> "nearestConnector"?

these two methods take arguments and are not normal properties so they
retain the 'get' prefix.

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

« Back to merge proposal