Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate offloadNever() #2133

Merged
merged 7 commits into from
Mar 15, 2022
Merged

Deprecate offloadNever() #2133

merged 7 commits into from
Mar 15, 2022

Conversation

bondolo
Copy link
Contributor

@bondolo bondolo commented Mar 8, 2022

Motivation:
Currently there is a special HttpExecutionStrategy, offloadNever(), that
has unusual behavior when merged with other HttpExecutionStrategys,
specifically the merge() always returns the offloadNever() strategy. To
reduce complexity and make the behavior of merge more consistent the
special merge behavior will be eliminated and this special strategy will be
deprecated for eventual removal.
Modifications:
Replace uses of offloadNever(), where possible with offloadNone()
and eliminate dependence of ServiceTalk implementation on the current
special merge behavior. Deprecate the special offloadNever() strategy.
Result:
Simplified implementation with more consistent behavior.

bondolo added 3 commits March 8, 2022 13:55
Motivation:
Currently there is a special `HttpExecutionStrategy` that has unusual
behavior when merged with other `HttpExecutionStrategy`s, specifically
the `merge()` always returns the `offloadNever()` strategy. To reduce
complexity and make the behavior of merge more consistent the special
merge behavior will be eliminated.
Modifications:
Replace uses of `offloadNever()`, where possible with `offloadNone()`
and eliminate dependence of ServiceTalk implementation on the current
special merge behavior.
Result:
Simplified implementation with more consistent behavior.
@bondolo bondolo added the technical debt Reduces technical debt label Mar 8, 2022
@bondolo bondolo self-assigned this Mar 8, 2022
@bondolo bondolo changed the title Remove special merge behavior for offloadNever() Deprecate offloadNever() Mar 8, 2022
Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!
Minor comments and 🚢

@bondolo bondolo merged commit d62bfd2 into apple:main Mar 15, 2022
@bondolo bondolo deleted the deprecate-offloadNever branch March 15, 2022 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical debt Reduces technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants