← Back to team overview

yellow team mailing list archive

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