Code review comment for lp:~frankban/juju-gui/bug-1081803-tests-mutate-url

Revision history for this message
Nicola Larosa (teknico) wrote :

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/

« Back to merge proposal