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

Revision history for this message
Jelmer Vernooij (jelmer) 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!
Thanks. I've fixed the issues you've mentioned and cleaned up the lint in the files I've changed.

Is there an easy way to lint all affected files, other than through the lpreview-body plugin?

« Back to merge proposal