Don't call handle_file() if a patched file can't be fetched.

Review Request #11547 — Created March 22, 2021 and submitted

Information

ReviewBot
release-3.0.x

Reviewers

All tools that implement handle_file() were calling
get_patched_file_path() on the reviewed file, and returning early if
it was None. Rather than do this in each handle_file() method, it
makes more sense to do it in handle_files(), before calling
handle_file().

This change switches to that, and passes in the resulting path as a
path variable, so handle_file() can operate on it.

We don't perform the check or pass in path for legacy tools, since the
old handle_file() didn't accept **kwargs and wouldn't expect it.

To handle the legacy tool check, the old base classes now set a
legacy_tool attribute, which the inner class can check. This is
admittedly pretty hacky, but won't exist past Review Bot 3.0, and is
easier to maintain than forking these methods into the old base classes.

Unit tests pass on Python 2.7 and 3.x.

Summary ID
Don't call handle_file() if a patched file can't be fetched.
All tools that implement `handle_file()` were calling `get_patched_file_path()` on the reviewed file, and returning early if it was `None`. Rather than do this in each `handle_file()` method, it makes more sense to do it in `handle_files()`, before calling `handle_file()`. This change switches to that, and passes in the resulting path as a `path` variable, so `handle_file()` can operate on it. We don't perform the check or pass in `path` for legacy tools, since the old `handle_file()` didn't accept `**kwargs` and wouldn't expect it. To handle the legacy tool check, the old base classes now set a `legacy_tool` attribute, which the inner class can check. This is admittedly pretty hacky, but won't exist past Review Bot 3.0, and is easier to maintain than forking these methods into the old base classes.
bd11340cade03698c6ec60bd1a34e92cfe0422d6
Description From Last Updated

Can you add documentation for legacy_tool?

daviddavid
david
  1. 
      
  2. Show all issues

    Can you add documentation for legacy_tool?

    1. I can. It's really just internal state that shouldn't ever be touched or set directly.

  3. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (cb7d553)