launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08260
Re: [Merge] lp:~james-w/launchpad/suppress-notifications-for-all into lp:launchpad
Review: Approve code
This branch looks good. I spotted one small improvement you could make
and had a couple of thoughts about lint while reading over it.
The newly-dedented return on line 104 of the diff can now fit on one
line.
Lint:
- I concur that leaving the long lines for doctests is normally the
right thing to do.
- It looks like fixing the "too many blank lines" in archive.py
wouldn't hurt too much.
- I couldn't find the long line reported on line 2990 of person.py.
Maybe the lint report quoted was from a point in time when the
changed code was all on one line.
--
https://code.launchpad.net/~james-w/launchpad/suppress-notifications-for-all/+merge/107560
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References