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

Async support for when, whenError and onRetry #679

Merged
merged 2 commits into from
Apr 14, 2022
Merged

Async support for when, whenError and onRetry #679

merged 2 commits into from
Apr 14, 2022

Conversation

beroso
Copy link
Contributor

@beroso beroso commented Mar 16, 2022

Addresses issue #645 .

In a general way, it add support for assigning async callbacks to when, whenError and onRetry

@kevmoo
Copy link
Member

kevmoo commented Mar 16, 2022

Sadly, we cannot do this without releasing a major version. This is a breaking change.

CC @natebosch

@natebosch
Copy link
Member

I do think this looks like a nice change, but we can't do a major bump right now.

If you file an issue we can tag it so that it gets attention once we are ready to push through a major version.

@natebosch natebosch closed this Mar 16, 2022
@beroso
Copy link
Contributor Author

beroso commented Mar 16, 2022

Thanks for the feedback.

Sorry, but I thought it wasn't a breaking change because I'm using FutureOr. The previous tests are passing and also tried to subclass the RetryClient with these changes and it worked.

There is a chance to reconsider this PR, because in my opinion it is backwards compatible?

@kevmoo , @natebosch, could you show me an example of api breaking with these changes? Maybe I'm missing the point.

@natebosch
Copy link
Member

Any code with @override of these methods will break. This is non breaking for uses of the client, but it is breaking for implementations of the client. Both are supporting by this package.

@beroso
Copy link
Contributor Author

beroso commented Mar 17, 2022

@natebosch I don't get it, because I just changed the type of private variables, not methods. The private vars are not exposed by the API. And the implementations constructors continues to working properly.

Could you show a concrete example of breaking?

@natebosch
Copy link
Member

Ah, I think @kevmoo and I were mistaken that there were changed parameters on methods instead of constructors.

I do think this is non-breaking. I'll need to investigate whether it has impact on my current approach for #424

@natebosch natebosch reopened this Mar 17, 2022
@beroso
Copy link
Contributor Author

beroso commented Mar 17, 2022

Thank you!

@lrhn
Copy link
Member

lrhn commented Mar 21, 2022

I believe this is practically non-breaking (almost everything is technically breaking, but it requires people to go through some hoops to be broken).
Changes look good to me, so just need @natebosch to be fine too,

@natebosch natebosch merged commit a645b93 into dart-lang:master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants