yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

On 2013/01/10 00:32:12, matthew.scott wrote:
> This looks good to me, land with a few minors (or correct me if I'm
wrong!).

> Works well for me.


https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars
> File app/templates/login.handlebars (right):


https://codereview.appspot.com/7060066/diff/1/app/templates/login.handlebars#newcode8
> app/templates/login.handlebars:8: <input type="text"
disabled="disabled"
> value="admin"></input>
> In general, <a> and <script> are the only empty tags.  input (along
with hr, b,

Should be br, sorry!

> etc) is usually a void tag, meaning that it self-closes like <input
/>.  I
> belieeeeve that's what's done elsewhere?  I'll need to check, but I
suppose
> matching the current standard we use is best.


https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less
> File lib/views/stylesheet.less (right):


https://codereview.appspot.com/7060066/diff/1/lib/views/stylesheet.less#newcode1406
> lib/views/stylesheet.less:1406:
> Indenting is a mix of 2 and 4 here (which, to be fair, is the case for
the whole
> file.  Maybe this is something to look into in the future for the rest
of the
> file, but could be solidified for this bit below.

> https://codereview.appspot.com/7060066/diff/1/test/test_login.js
> File test/test_login.js (right):


https://codereview.appspot.com/7060066/diff/1/test/test_login.js#newcode119
> test/test_login.js:119: test('the view login method calls the
environment login
> one', function() {
> The 'one' at the end of the test description is ambiguous; thought it
was
> referring to view, which may be due to our dual use of 'environment'
for the
> view and the WS layer.  'the view login method logs in through the
environment'
> maybe?



https://codereview.appspot.com/7060066/

-- 
https://code.launchpad.net/~gary/juju-gui/login-ui/+merge/142599
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/login-ui into lp:juju-gui.