Remove deprecated Temporal.configuration function#343
Open
aguynamedben wants to merge 1 commit intocoinbase:masterfrom
Open
Remove deprecated Temporal.configuration function#343aguynamedben wants to merge 1 commit intocoinbase:masterfrom
Temporal.configuration function#343aguynamedben wants to merge 1 commit intocoinbase:masterfrom
Conversation
Fixes coinbase#143. For a few years there has been a noise `warn` message telling people to not use the function Temporal.configuration, but this function was still used internall in temporal-ruby. The options were: a) Remove the logger warn message, as whoever in the past had planned to deprecate access via `Temporal.configuration` never followed up. I don't know why they were trying to remove this method of accessing it. b) **[CHOSEN OPTION]** Raise the `config` function from `private` to be accessible externally. I don't see why this wouldn't be okay. I ran the tests per the instructions and I get the same amount of test passes, without the noisy deprecation warnings. I don't know the full history that led to this being half-deprecated, but this seems like a fine way to clean it up.
aguynamedben
commented
Jun 28, 2025
|
|
||
| def config | ||
| @config ||= Configuration.new | ||
| end |
Author
There was a problem hiding this comment.
Not removed, hoisted out of private ⬆️
Author
|
Bump! @cj-cb |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #143. For a few years there has been a noise
warnmessage telling people to not use the function Temporal.configuration, but this function was still used internall in temporal-ruby.The options were:
a) Remove the logger warn message, as whoever in the past had planned to deprecate access via
Temporal.configurationnever followed up. I don't know why they were trying to remove this method of accessing it. b) [CHOSEN OPTION] Raise theconfigfunction fromprivateto be accessible externally. I don't see why this wouldn't be okay.I ran the tests per the instructions and I get the same amount of test passes, without the noisy deprecation warnings.
I don't know the full history that led to this being half-deprecated, but this seems like a fine way to clean it up.
Before
After
Breaking Change
CHANGELOG.md should have:
Temporal.configurationwas removed and replace byTemporal.config. If you previously relied onTemporal.configurationjust update the reference toTemporal.config.Also if this Gem is still doing any versioning, it's a MAJOR patch (i.e. first digit) because if removes a public-facing API, i.e.
Temporal.configuration. If we don't want to create a breaking change, I recommend Option A above, just remove the annoying log message and let the method live on into the future.