yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #02138
Re: Add user login support. (issue 7007047)
This looks ok to me, given the changes Kapil requests. I'll look again
after those changes are made.
https://codereview.appspot.com/7007047/diff/1/app/app.js
File app/app.js (right):
https://codereview.appspot.com/7007047/diff/1/app/app.js#newcode530
app/app.js:530: * @param {Object} res ???
Why the duplicate 'res' and the ???
https://codereview.appspot.com/7007047/diff/1/app/views/login.js
File app/views/login.js (right):
https://codereview.appspot.com/7007047/diff/1/app/views/login.js#newcode3
app/views/login.js:3: var no_login_prompts = false;
Why not camelCase?
https://codereview.appspot.com/7007047/diff/1/app/views/login.js#newcode3
app/views/login.js:3: var no_login_prompts = false;
Why not camelCase?
https://codereview.appspot.com/7007047/diff/1/app/views/login.js#newcode27
app/views/login.js:27: * React to the results of sennding a login
message to the server.
typo: sending
https://codereview.appspot.com/7007047/diff/1/test/test_login_view.js
File test/test_login_view.js (right):
https://codereview.appspot.com/7007047/diff/1/test/test_login_view.js#newcode147
test/test_login_view.js:147: // If there are know credentials that are
not known to be bad (they are
s/know/no/
https://codereview.appspot.com/7007047/
--
https://code.launchpad.net/~benji/juju-gui/login/+merge/141133
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~benji/juju-gui/login into lp:juju-gui.
References