Pass tool settings at construction time, and emit deprecation warnings.

Review Request #11532 — Created March 18, 2021 and submitted

Information

ReviewBot
release-3.0.x

Reviewers

Previously, a tool run involved creating the Tool subclass and then
calling a handful of methods on it, each time passing in a settings
argument.

This change simplifies this, instead having the new BaseTool take
settings on construction, so that any method can access it. That
avoids more complicated method signatures going forward.

This in turn deprecates the settings argument on all these methods.
To help transition to this, and avoid issues in the future, these
methods are now all expected to take a **kwargs argument. This allows
us to continue passing in settings for now, and for older tools to
support it, but for newer ones to ignore it.

To transition people to the new behavior, legacy Tool and
RepositoryTool construction (via __new__) now emits a deprecation
warning telling tool authors what must be done to transition their tool.
We're using dedicated deprecation classes in a reviewbot.deprecation
module to ease this and future deprecations, the same way we do with
Review Board and Djblets.

All of our current tools are considered deprecated, and will emit a
warning. Those will be updated one-by-one.

Unit tests pass on Python 2.7 and 3.x.

Ran the worker, refreshed tools, and triggered new code reviews, without
any issues. Saw the deprecation warnings for our tools on launch.

Summary ID
Pass tool settings at construction time, and emit deprecation warnings.
Previously, a tool run involved creating the `Tool` subclass and then calling a handful of methods on it, each time passing in a `settings` argument. This change simplifies this, instead having the new `BaseTool` take `settings` on construction, so that any method can access it. That avoids more complicated method signatures going forward. This in turn deprecates the `settings` argument on all these methods. To help transition to this, and avoid issues in the future, these methods are now all expected to take a `**kwargs` argument. This allows us to continue passing in `settings` for now, and for older tools to support it, but for newer ones to ignore it. To transition people to the new behavior, legacy `Tool` and `RepositoryTool` construction (via `__new__`) now emits a deprecation warning telling tool authors what must be done to transition their tool. We're using dedicated deprecation classes in a `reviewbot.deprecation` module to ease this and future deprecations, the same way we do with Review Board and Djblets. All of our current tools are considered deprecated, and will emit a warning. Those will be updated one-by-one.
a7cbd243763ac09077185b23abe1527fb775f277
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (a4e2ea1)