Code review comment for lp:~jjacobs/methanal/verified-password-input

Revision history for this message
Jonathan Jacobs (jjacobs) wrote :

> 1. Saving and restoring control._confirmPasswordNode seems pointless; nothing
> relies on this behaviour, and the first password input isn't saved and
> restored, so it's somewhat inconsistent.

This behaviour has been removed, it didn't seem to matter to anything, I guess if you need a clean slate, write another test case.

> 2. In general, the pattern "throw new Error('Foo bar:' + c);" is better
> written as "throw new FooBar(c);"; embedding the exception "class" in text
> makes it challenging to catch the specific exception without doing something
> yucky like substring matching. In this specific case, setStrengthCriteria
> should probably throw UnknownStrengthCriterion or InvalidStrengthCriterion or
> something like that.

I was being lazy.

« Back to merge proposal