yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

Thanks!


https://codereview.appspot.com/6782063/diff/14001/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6782063/diff/14001/app/views/environment.js#newcode1246
app/views/environment.js:1246: */
On 2012/11/13 16:41:48, benji wrote:
> A @return entry would be nice. We have been using the recommended
"@return
> {undefined}" for methods that do not return anything.

Done.

https://codereview.appspot.com/6782063/diff/14001/app/views/environment.js#newcode1249
app/views/environment.js:1249:
this.addSVGClass(Y.one(subordinate).one('.sub-rel-count'), 'active');
On 2012/11/13 16:41:48, benji wrote:
> If "subordinate" is already a node, why do we have to do the
> "Y.one(subordinate)" dance?  Is it because it is a bare DOM node and
we need a
> YUI node?  If so, it might be good if the parameter comment included
that fact.

Done.

https://codereview.appspot.com/6782063/diff/14001/app/views/environment.js#newcode1256
app/views/environment.js:1256: */
On 2012/11/13 16:41:48, benji wrote:
> @return

Done.

https://codereview.appspot.com/6782063/diff/14001/app/views/environment.js#newcode1260
app/views/environment.js:1260:
this.removeSVGClass(Y.one('.sub-rel-count.active'), 'active');
On 2012/11/13 16:41:48, benji wrote:
> Using a non-scoped Y.one() like this will be a problem if we ever have
more than
> one environment view on a page, but I suppose that is not something we
should
> worry about right now.

Easy fix.  Done.

https://codereview.appspot.com/6782063/

-- 
https://code.launchpad.net/~makyo/juju-gui/remove-sub-rels/+merge/131725
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/remove-sub-rels into lp:juju-gui.