Thanks for the review, Rick! The test helper is following the pattern of setupBrowserForUser, which takes a real user. Not sure I want to get into all that refactoring. By refactoring, I mean that I don't like those kind of 1/2 refactors where there are now 2 ways to do something. Anyway, it's a good suggestion, but I'll wait on doing it for now.
As for line #515, I think that's just a style preference and we don't have a rule about being explicit there, I don't think. I prefer the return by itself, rather than inside an else statement. Less code and all, for the same result. For the timing question, I wondered even if it should be a config value, but wasn't sure. So I'll keep thinking on that, but roll on forward for now. If I changed it, I'll do it in one of the follow on branches. Thanks, again, for the review! -- https://code.launchpad.net/~deryck/launchpad/reauth-for-email-363916/+merge/118612 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
