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

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

  1. Some empty lines have trailing whitespace, please remove this.
  2. Can you align the second line of the 3rd point of "test_inputValidation"'s docstring with the first line?
  3. "test_inputValidation" tests a bunch of different cases, these probably don't need to be split into separate methods but if you could add a comment to indicate which condition is being tested, that would help.
  4. Some lines (excluding subclass lines) are wider than 80 columns.
  5. Please use braces for *ALL* blocks, regardless of the length of their body. "VerifiedPasswordInput.baseValidator" is one specific case I'm looking at.
  6. Replacing the template for things that inherit from "TextInput" makes me twitch, I really don't like this. Unfortunately in this case it doesn't seem to be easily avoidable. The template "methanal-verified-password-input.html" is missing a number of elements that TextInput could be expecting, can you copy "methanal-text-input.html" and modify whatever needs to be?

review: Needs Fixing

« Back to merge proposal