← Back to team overview

launchpad-reviewers team mailing list archive

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