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

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

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.

Commits

Files