yellow team mailing list archive

[Nicola Larosa <nicola.larosa@xxxxxxxxxxxxx>]

Nice cleanup, just a couple minor comments inline.


https://codereview.appspot.com/6856075/diff/1/test/test_app.js
File test/test_app.js (left):

https://codereview.appspot.com/6856075/diff/1/test/test_app.js#oldcode50
test/test_app.js:50: }
I'm a bit concerned about this: I wonder what exactly "losing the race"
means. It might warrant further investigation.

https://codereview.appspot.com/6856075/diff/1/test/test_app_hotkeys.js
File test/test_app_hotkeys.js (right):

https://codereview.appspot.com/6856075/diff/1/test/test_app_hotkeys.js#newcode41
test/test_app_hotkeys.js:41: keyCode: 83, //  "S" key.
Is the additional space before the "S" useful?

https://codereview.appspot.com/6856075/diff/1/test/test_app_hotkeys.js#newcode58
test/test_app_hotkeys.js:58: keyCode: 69, //  "E" key.
Same comment as line 41 above.

https://codereview.appspot.com/6856075/

-- 
https://code.launchpad.net/~frankban/juju-gui/bug-1081803-tests-mutate-url/+merge/135670
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/juju-gui/bug-1081803-tests-mutate-url into lp:juju-gui.