File modification checking multi-line support and cleanup

Review Request #10280 — Created Oct. 25, 2018 and submitted

Information

ReviewBot
master
502f6f1...

Reviewers

Multiple TODOs have been addressed in _is_modified, so that tools
commenting on multiple lines have modifications properly checked. The
algorithm has been made more efficient, only checking relevant
(modified) chunks to see if they are in the range of modification.

A patch was also made to tools that 'globally comment' (I.E. make a
comment with a line of <=0). Previously, all this would do is round the
line number up to 1, (as the front-end does not support rendering
first_line=None), which had the unintended side-effect of tools
configured to only comment on modified code who make global comments, to
only have their comment go through if line 1 was modified. The
first_line=1 workaround is still used, and the file is assumed to be
modified.

I've tested _is_modified through my nyc plugin directly, and
tested File.comment() manually through a stub-plugin. Waiting on test
suite creation to mock up standalone tests.

Description From Last Updated

"ReviewBoard" -> "Review Board"

daviddavid

We generally avoid Python's ternary form because it's confusing and ugly.

daviddavid

Can we have one blank line above this?

daviddavid

This comment doesn't really make sense.

daviddavid

``None``

brenniebrennie
david
  1. 
      
  2. bot/reviewbot/processing/review.py (Diff revision 1)
     
     
    Show all issues

    "ReviewBoard" -> "Review Board"

  3. bot/reviewbot/processing/review.py (Diff revision 1)
     
     
    Show all issues

    We generally avoid Python's ternary form because it's confusing and ugly.

  4. bot/reviewbot/processing/review.py (Diff revision 1)
     
     
    Show all issues

    Can we have one blank line above this?

  5. 
      
alextechcc
alextechcc
ilaw
  1. 
      
  2. bot/reviewbot/processing/review.py (Diff revision 3)
     
     

    Should the documentation for the line_num parameter also be changed to be re: checking and not comments?

  3. 
      
david
  1. 
      
  2. bot/reviewbot/processing/review.py (Diff revision 3)
     
     
    Show all issues

    This comment doesn't really make sense.

  3. 
      
alextechcc
brennie
  1. LGTM

  2. bot/reviewbot/processing/review.py (Diff revision 4)
     
     
    Show all issues

    ``None``
    
  3. 
      
alextechcc
david
  1. Ship It!
  2. 
      
alextechcc
Review request changed
Status:
Completed
Change Summary:
Pushed to master (8ad985e)