Skip to content

Commit

Permalink
Tweak deprecation messages. (#15027)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kaos authored Apr 7, 2022
1 parent 68aca67 commit 902d3ac
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 9 deletions.
6 changes: 3 additions & 3 deletions src/python/pants/base/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions src/python/pants/base/deprecated_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/option/options_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/option/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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


Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/option/subsystem_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

0 comments on commit 902d3ac

Please sign in to comment.