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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
