Skip to content

Remove and replace deprecated APIs marked for removal in 2.0.x #1209

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

Merged
merged 1 commit into from
Aug 14, 2025

Conversation

therepanic
Copy link
Contributor

Due to gh-1187, we should remove deprecates that are marked for removal from version 1.2.0.

Closes: gh-1187

@therepanic
Copy link
Contributor Author

Strangely, not all PulsarListenerTxnTests tests fail, only throwsExceptionWhenTransactionsAreRequired and ignoresSettingWhenNoTxnManagerAvailable. Any ideas what this could be? We register PulsarAutoConfiguration, and it seems like it should work as expected.

@onobc
Copy link
Collaborator

onobc commented Aug 10, 2025

Strangely, not all PulsarListenerTxnTests tests fail, only throwsExceptionWhenTransactionsAreRequired and ignoresSettingWhenNoTxnManagerAvailable. Any ideas what this could be? We register PulsarAutoConfiguration, and it seems like it should work as expected.

I will take a look but the txn tests are flaky and I have been meaning to dig in and understand why. It has not been obvious so far. I would first try to re-run failed in the GH actions and see if it passes.

Copy link
Collaborator

@onobc onobc left a comment

Choose a reason for hiding this comment

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

Thanks for removing the deprecated APIs @therepanic - super helpful!

I have one suggestion around the customizer / txn tests. Also, do you mind updating the WhatsNew.adoc and start a 2.0 -> 1.2 section w/ these removed APIs?

@therepanic therepanic force-pushed the gh-1187 branch 3 times, most recently from 921098f to 86b43be Compare August 13, 2025 23:24
@therepanic
Copy link
Contributor Author

Thanks for the review, @onobc.

The idea with PulsarTxnTestsBase is really good, I implemented it.

Also, thanks for the suggestion about updating whats-new.adoc, I did that. Is it possible not to rebase this commit and push it as a separate one? Does it make sense or is it still worth rebasing?

@therepanic therepanic requested a review from onobc August 13, 2025 23:47
@onobc
Copy link
Collaborator

onobc commented Aug 14, 2025

Thanks for the review, @onobc.

The idea with PulsarTxnTestsBase is really good, I implemented it.

Also, thanks for the suggestion about updating whats-new.adoc, I did that. Is it possible not to rebase this commit and push it as a separate one? Does it make sense or is it still worth rebasing?

Go ahead and squash and rebase. I have reviewed the PR and it is good. You will need to rebase as the checkstyle has changed due to the license header updates we recently implemented. Thank you.

Copy link
Collaborator

@onobc onobc left a comment

Choose a reason for hiding this comment

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

After the 2 minor fixes for checkstyle this one is good to go. Nice work! Thanks again.

@therepanic therepanic force-pushed the gh-1187 branch 2 times, most recently from 342354a to 9380b77 Compare August 14, 2025 11:23
@therepanic therepanic requested a review from onobc August 14, 2025 11:33
@onobc
Copy link
Collaborator

onobc commented Aug 14, 2025

Hi @therepanic , I added a commit to your PR as the code needed to be in the already existing PulsarTxnTestsBase. After this it passed locally. I expect this to be green and then I will get it merged. Thanks for banging this one out - much appreciated.

This commit removes APIs that were marked for removal in version 1.2.0.

Also updates the `whats-new.adoc` with the list of removed APIs.

Fixes: spring-projects#1187

Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
@onobc onobc merged commit 81428c9 into spring-projects:main Aug 14, 2025
7 checks passed
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.

Replace "forRemoval" deprecations in 2.0.x
2 participants