Thanks for the re-review Benji.
https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js File app/views/charm-panel.js (right): https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js#newcode82 app/views/charm-panel.js:82: * @return {Array} A filtered, grouped set of filtered entries. On 2012/12/03 20:58:15, benji wrote: > So, is the return value filtered? ;P YES, I wanted to make sure it was well known! fixed https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js#newcode93 app/views/charm-panel.js:93: var isSubFilter = function(charm) { On 2012/12/03 20:58:15, benji wrote: > Why not just "function isSubFilter"? Oh, wait, I think there is a bug in some > browsers that makes that not work right. Is that it? no, it was just plain silliness. fixed. we'll see now if i break those browsers. https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js#newcode980 app/views/charm-panel.js:980: // "charms" panel. On 2012/12/03 20:58:15, benji wrote: > I don't feel strongly one way or the other (or both, even), but I thought I > would point out that some of the code in this branch uses /* */ style comments > and some uses // comments. I'll fix it so that YUIDOC are /** */ and others are //. https://codereview.appspot.com/6842119/diff/12001/test/test_charm_panel.js File test/test_charm_panel.js (right): https://codereview.appspot.com/6842119/diff/12001/test/test_charm_panel.js#newcode326 test/test_charm_panel.js:326: .setAttribute('id', 'charm-search-test').append( On 2012/12/03 20:58:15, benji wrote: > Since this code is chaining method calls, shouldn't the ".append" be the first > code on a new line? Like so: http://paste.ubuntu.com/1408800/ Done. https://codereview.appspot.com/6842119/ -- https://code.launchpad.net/~bac/juju-gui/cs-filter/+merge/137050 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/juju-gui/cs-filter 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
