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

Add publishOn/subscribeOn Executor overload #2155

Merged
merged 3 commits into from
Mar 18, 2022

Conversation

bondolo
Copy link
Contributor

@bondolo bondolo commented Mar 18, 2022

Motivation:
The signature of publishOn(Executor) and subscribeOn(Executor)
methods changed between 0.41 and 0.42 from
io.servicetalk.concurrent.api.Executor to the more general parent
type io.servicetalk.concurrent.api.Executor. This is a source
compatible change since the new type is a parent of the old type, but
is not a binary compatible change; code compiled against 0.41 will not
run with 0.42 as it will not find the method it expects with the
io.servicetalk.concurrent.api.Executor signature.
Modifications:
Add overloads publishOn(Executor) and subscribeOn(Executor) for
io.servicetalk.concurrent.Executor. For binary compatibility with
0.42 it will also be necessary to cast executors to
io.servicetalk.concurrent.api.Executor to ensure that the correct
overload is used.
Result:
Improved compatibility with ServiceTalk 0.42

Motivation:
The signature of `publishOn(Executor)` and `subscribeOn(Executor)`
methods changed between 0.41 and 0.42 from
`io.servicetalk.concurrent.api.Executor` to the more general parent
type `io.servicetalk.concurrent.api.Executor`. This is a source
compatible change since the new type is a parent of the old type, but
is not a binary compatible change; code compiled against 0.41 will not
run with 0.42 as it will not find the method it expects with the
`io.servicetalk.concurrent.api.Executor` signature.
Modifications:
Add overloads `publishOn(Executor)` and `subscribeOn(Executor)` for
`io.servicetalk.concurrent.Executor`. For binary compatibility with
0.42 it will also be necessary to cast executors to
`io.servicetalk.concurrent.api.Executor` to ensure that the correct
overload is used.
Result:
Improved compatibility with ServiceTalk 0.42
@bondolo bondolo added enhancement New feature or request API PR with API changes (New or Deprecated) labels Mar 18, 2022
@bondolo bondolo self-assigned this Mar 18, 2022
* @param executor {@link Executor} to use.
* @return A new {@link Completable} that will use the passed {@link Executor} to invoke all methods of
* {@link Cancellable} and {@link #handleSubscribe(CompletableSource.Subscriber)}.
* @deprecated For compatibility with 0.42 convert to using {@link #publishOn(io.servicetalk.concurrent.Executor)}
Copy link
Member

Choose a reason for hiding this comment

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

publishOn -> subscribeOn

* @param executor {@link Executor} to use.
* @return A new {@link Publisher} that will use the passed {@link Executor} to invoke all methods of
* {@link Subscription} and {@link #handleSubscribe(PublisherSource.Subscriber)}.
* @deprecated For compatibility with 0.42 convert to using {@link #publishOn(io.servicetalk.concurrent.Executor)}
Copy link
Member

Choose a reason for hiding this comment

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

publishOn -> subscribeOn

* @param executor {@link Executor} to use.
* @return A new {@link Single} that will use the passed {@link Executor} to invoke all methods of
* {@link Cancellable} and {@link #handleSubscribe(SingleSource.Subscriber)}.
* @deprecated For compatibility with 0.42 convert to using {@link #publishOn(io.servicetalk.concurrent.Executor)}
Copy link
Member

Choose a reason for hiding this comment

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

publishOn -> subscribeOn

* @param executor {@link Executor} to use.
* @return A new {@link Completable} that will use the passed {@link Executor} to invoke all methods on the
* {@link Subscriber}.
* @deprecated For compatibility with 0.42 convert to using {@link #publishOn(io.servicetalk.concurrent.Executor)}
Copy link
Member

Choose a reason for hiding this comment

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

For compatibility with 0.42 convert to using {@link #publishOn(io.servicetalk.concurrent.Executor)} instead. -> Use {@link #publishOn(io.servicetalk.concurrent.Executor)}.. Consider minimizing the dialog here, deprecation implies it is removed in future releases.

@Scottmitch
Copy link
Member

feel free to merge after comments addressed.

Copy link
Member

@Scottmitch Scottmitch left a comment

Choose a reason for hiding this comment

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

I didn't investigate all usages where casting would be required, but assume you covered them.

@bondolo
Copy link
Contributor Author

bondolo commented Mar 18, 2022

I didn't investigate all usages where casting would be required, but assume you covered them.

I converted only one set of usages in the current tests to ensure that the new code was exercised, but otherwise left all of the other references in 0.41 using the deprecated method.

@bondolo bondolo merged commit 00061db into apple:0.41 Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API PR with API changes (New or Deprecated) enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants