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

Migrate from SettableFuture to CompletableFuture in tests #444

Merged
merged 1 commit into from
Apr 17, 2021

Conversation

basil
Copy link
Member

@basil basil commented Apr 5, 2021

According to this guide, SettableFuture.create() can be replaced with Java 8's new CompletableFuture(). I did so for any test code that used SettableFuture. Note that modifying usages of SettableFuture in non-test code has binary compatibility implications, so I did not touch non-test code in order to eliminate any risk of regression.

@oleg-nenashev oleg-nenashev added the chore For changelog: A maintenance chore with no functional changes label Apr 5, 2021
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Looks good to me, esp. since there are no production code changes

@oleg-nenashev
Copy link
Member

Hi @jeffret-b . Would you be fine if I start merging some of the Remoting pull requests?

@jeffret-bg
Copy link

@oleg-nenashev Because of the complex nature of testing and fully releasing / integrating Remoting I prefer a different approach. I've found it to work better when dealing with releases.

When a PR is ready, I mark it as ready-to-merge. That means it's sufficiently reviewed and tested and doesn't currently have any outstanding concerns. There are currently two PRs marked that way.

Then when I get ready to collect things into a release I pick up one or more ready-to-merge PRs, merge them in, and then start running through the testing and release cycle.

I've found a number of times where there was a higher priority change that came in later that needed to get out before another, less reliable change.

So, go ahead and mark things ready-to-merge.

I've thought about trying to get things assembled for a release. Not sure I would get to that this week. If someone else wants to help put things together for a release, that would be fine.

Copy link
Contributor

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the submission. Nice cleanup.

@oleg-nenashev oleg-nenashev merged commit 3ecc840 into jenkinsci:master Apr 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore For changelog: A maintenance chore with no functional changes ready-to-merge
Projects
None yet
5 participants