Skip to content

fix ci error on jdk17#208

Closed
XenoAmess wants to merge 1 commit intoapache:masterfrom
xenoamess-fork:fix_ci_error_on_jdk17
Closed

fix ci error on jdk17#208
XenoAmess wants to merge 1 commit intoapache:masterfrom
xenoamess-fork:fix_ci_error_on_jdk17

Conversation

@XenoAmess
Copy link
Contributor

@XenoAmess XenoAmess commented Feb 23, 2021

As title.
Not a big thing, so do not think need a JIRA ticket.,

@XenoAmess
Copy link
Contributor Author

@garydgregory please review. thanks. this one should be very easy to review.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.257% when pulling 54e10c4 on xenoamess-fork:fix_ci_error_on_jdk17 into 6db5a07 on apache:master.

@garydgregory
Copy link
Member

Is the mix of changing mockito AND adding a Maven profile intentional?

@garydgregory
Copy link
Member

Hi @XenoAmess

-1 for several reasons:

Nits: The title of this PR ("fix ci error on jdk17") is misleading as (1) the PR does not fix anything, it just ignores a part of the build and (2) this is also for an Early Access Java build (jdk17-ea) and not a released Java version of 17 which is not there yet.

Main: The 17-ea build is marked in our GitHub action as "experimental" and is allowed to fail by definition. The whole point (in my mind at least) for testing on EA builds is exactly what we are seeing now: a failure. In this case, it is quite nice to see that JaCoCo is not ready for Java 17 yet; this lets anyone who cares about the FOSS ecosystem go and help JaCoCo get up to speed on Java 17. If by these efforts we can get a new release of JaCoCo out sooner for us EA adventurers, then we are all better off.

@XenoAmess XenoAmess closed this Feb 24, 2021
@XenoAmess
Copy link
Contributor Author

Is the mix of changing mockito AND adding a Maven profile intentional?

yes. that is needed as mockito use older version of buddy which cannot deal with jdk17 binary

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