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

CURATOR-624: Fix Github Actions by adding 'distribution' for JDK setup step #407

Merged
merged 4 commits into from
Feb 25, 2022

Conversation

martin-g
Copy link
Member

Merge the two workflows into one and use strategy.matrix for the Java
versions.

Merge the two workflows into one and use strategy.matrix for the Java
versions

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@eolivelli
Copy link
Contributor

@nicoloboschi do you mind taking a look here ?

Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

I agree we have to use 'temurin'

Good work, I will close mine

@martin-g
Copy link
Member Author

It seems with one additional minor change (upgrade Mockito-core to 4.3.1) and few import fixes the build will pass with JDK 17 too.
Do you want me to do it in this PR ?

Upgrade Mockito (for JDK 17) and JUnit 5

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Copy link
Member Author

I guess the test failures are expected.
At least the CI setup now works!

@martin-g martin-g changed the title Fix Github Actions by adding 'distribution' for JDK setup step CURATOR-624: Fix Github Actions by adding 'distribution' for JDK setup step Feb 25, 2022
@eolivelli
Copy link
Contributor

It seems with one additional minor change (upgrade Mockito-core to 4.3.1) and few import fixes the build will pass with JDK 17 too.
Do you want me to do it in this PR ?

this would be great, thanks

@eolivelli
Copy link
Contributor

Maybe if you remove the jdk17 part and we see ci passing we can merge this pr and unblock the pipeline

Does it sound like a good plan to you?

@martin-g
Copy link
Member Author

martin-g commented Feb 25, 2022 via email

@eolivelli
Copy link
Contributor

Let's remove jdk17 stuff and merge this pr

@eolivelli
Copy link
Contributor

Then we can commit your fix to the test and add back 17

@martin-g
Copy link
Member Author

OK. The last commit is reverted!
I will create a new PR for it!

@martin-g martin-g closed this Feb 25, 2022
@martin-g martin-g reopened this Feb 25, 2022
@eolivelli eolivelli merged commit 6f7b3e7 into apache:master Feb 25, 2022
@martin-g martin-g deleted the fix-github-actions branch May 4, 2022 06:41
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.

3 participants