launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

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.