Code review comment for lp:~jelmer/launchpad/621778-parse-homepage-field

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Looks good. Just two notes:

In line 49 of the diff: don't bother documenting tests as "Test that…" That's obvious from them being test methods. Just takes up space.

In line 94 of the diff: badly formatted line break. Try putting both arguments to the assertEqual together on the line after the invocation.

Please mention the "make lint" output in your merge proposals so that your reviewer doesn't need to worry about whether they have to play human linter. If there's too much pre-existing lint, well, throw in some drive-by cleanups!

review: Approve (code)

« Back to merge proposal