From 902d3acb84c1b64c53c05e882200e4d578884c03 Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Thu, 7 Apr 2022 22:22:09 +0200 Subject: [PATCH] Tweak deprecation messages. (#15027) With the push of removing deprecated features, the wording for the deprecated features are slightly inaccurate when stating `"DEPRECATED: {entity} will be removed in version 2.12.0.dev0"`, when later pushed to `.dev1` and again to `.dev2`. Suggest to soften this to give the intent of removal, without the actual promise of removal. `"DEPRECATED: {entity} may be removed in version 2.12.0.dev0"` Considered dropping the pre release part, so it reads `"... will be removed in version 2.12.0"`, which I feel is a better user facing message, but I was reluctant to trim the actual `removal_version` presented. Also found a couple of mixups with the format strings for two deprecation error messages. --- src/python/pants/base/deprecated.py | 6 +++--- src/python/pants/base/deprecated_test.py | 6 ++++-- src/python/pants/option/options_integration_test.py | 2 +- src/python/pants/option/options_test.py | 4 ++-- src/python/pants/option/subsystem_test.py | 5 ++++- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/python/pants/base/deprecated.py b/src/python/pants/base/deprecated.py index 9b14ef5d057..0e24f67bfb0 100644 --- a/src/python/pants/base/deprecated.py +++ b/src/python/pants/base/deprecated.py @@ -57,7 +57,7 @@ def is_deprecation_active(start_version: str | None) -> bool: def get_deprecated_tense(removal_version: str) -> str: """Provides the grammatical tense for a given deprecated version vs the current version.""" - return "will be" if (Version(removal_version) >= PANTS_SEMVER) else "was" + return "is scheduled to be" if (Version(removal_version) >= PANTS_SEMVER) else "was" @memoized_method @@ -72,11 +72,11 @@ def validate_deprecation_semver(version_string: str, version_description: str) - :raises DeprecationError: if the version_string parameter is invalid. """ if version_string is None: - raise MissingSemanticVersionError(f"The {version_string} must be provided.") + raise MissingSemanticVersionError(f"The {version_description} must be provided.") if not isinstance(version_string, str): raise BadSemanticVersionError( f"The {version_description} must be a version string but was {version_string} with " - f"type {type(version_description)}." + f"type {type(version_string)}." ) try: diff --git a/src/python/pants/base/deprecated_test.py b/src/python/pants/base/deprecated_test.py index 95f30becd08..daa93144a0a 100644 --- a/src/python/pants/base/deprecated_test.py +++ b/src/python/pants/base/deprecated_test.py @@ -90,7 +90,7 @@ def test_deprecated_module(caplog) -> None: assert not caplog.records deprecated_module(FUTURE_VERSION, hint="Do not use me.") assert len(caplog.records) == 1 - assert "module will be removed" in caplog.text + assert "module is scheduled to be removed" in caplog.text assert "Do not use me" in caplog.text @@ -179,7 +179,9 @@ def test_deprecation_start_period(caplog) -> None: start_version=_FAKE_CUR_VERSION, ) assert len(caplog.records) == 1 - assert "DEPRECATED: demo will be removed in version 999.999.999.dev999." in caplog.text + assert ( + "DEPRECATED: demo is scheduled to be removed in version 999.999.999.dev999." in caplog.text + ) @unittest.mock.patch("pants.base.deprecated.PANTS_SEMVER", Version(_FAKE_CUR_VERSION)) diff --git a/src/python/pants/option/options_integration_test.py b/src/python/pants/option/options_integration_test.py index f9777bf0221..30c99965907 100644 --- a/src/python/pants/option/options_integration_test.py +++ b/src/python/pants/option/options_integration_test.py @@ -84,7 +84,7 @@ def rules(): result.assert_success() assert unmatched_glob_warning in result.stderr assert ( - "DEPRECATED: option 'deprecated' in scope 'mock-options' will be removed in version " + "DEPRECATED: option 'deprecated' in scope 'mock-options' is scheduled to be removed in version " "999.99.9.dev0." ) in result.stderr diff --git a/src/python/pants/option/options_test.py b/src/python/pants/option/options_test.py index 4ac9224c18c..b1bbe58af5d 100644 --- a/src/python/pants/option/options_test.py +++ b/src/python/pants/option/options_test.py @@ -350,7 +350,7 @@ def assert_deprecated( opts = create_options([GLOBAL_SCOPE, "scope"], register, args, env=env, config=config) assert opts.for_scope(scope)[opt] == expected assert len(caplog.records) == 1 - assert "will be removed in version" in caplog.text + assert "is scheduled to be removed in version" in caplog.text assert opt in caplog.text assert_deprecated(GLOBAL_SCOPE, "old1", ["--old1=x"], expected="x") @@ -416,7 +416,7 @@ def register(opts: Options) -> None: == "x" ) assert len(caplog.records) == 1 - assert "will be removed in version" in caplog.text + assert "is scheduled to be removed in version" in caplog.text assert "past_start" in caplog.text diff --git a/src/python/pants/option/subsystem_test.py b/src/python/pants/option/subsystem_test.py index 2fb33c376f2..281875d8fc2 100644 --- a/src/python/pants/option/subsystem_test.py +++ b/src/python/pants/option/subsystem_test.py @@ -90,4 +90,7 @@ def register_options(cls, register): ) OldAndDusty.register_options_on_scope(options) - assert "DEPRECATED: pants.option.subsystem.register_options() will be removed" in caplog.text + assert ( + "DEPRECATED: pants.option.subsystem.register_options() is scheduled to be removed" + in caplog.text + )