yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

If you want me to look at this again I would be happy to, but otherwise
land with changes.

Thank you

Gary


https://codereview.appspot.com/7007047/diff/15001/app/store/env.js
File app/store/env.js (right):

https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode87
app/store/env.js:87: this.set('defaultSeries', msg.default_series);
The above two lines are supposed to be added to handleLoginEvent, and
removed from here entirely later. Unfortunately, the current rapi
implementation does not support this.

https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode113
app/store/env.js:113: }
The implementation does not do what was specced, but we should look
forward to it anyway, I think.

else {
   if (evt.data.provider_type) {
     this.set('providerType', msg.data.provider_type);
   }
   if (evt.data.default_series) {
     this.set('defaultSeries', msg.data.default_series);
   }
}

In the future we should be able to remove the conditional parts of that,
and rely on the two values existing.

https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode223
app/store/env.js:223: window.setTimeout(Y.bind(this.login, this), 500);
We talked about this and you liked the idea of connecting this to an
event instead (event fired by login attempt).

https://codereview.appspot.com/7007047/diff/15001/test/index.html
File test/index.html (right):

https://codereview.appspot.com/7007047/diff/15001/test/index.html#newcode37
test/index.html:37: // *after* the test runner is invoked.  In which
case the test runner
"In this case,"

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.