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

Add a build job on JDK 8 #2480

Merged
merged 1 commit into from
Aug 23, 2021
Merged

Add a build job on JDK 8 #2480

merged 1 commit into from
Aug 23, 2021

Conversation

izeye
Copy link
Contributor

@izeye izeye commented Feb 26, 2021

This PR adds a build job on JDK 8 as CONTRIBUTING.md claims that Micrometer can be built against JDK 8, but there's no explicit check for it except the side effect from the docker-tests job.

See #2476 (comment)

@izeye izeye force-pushed the jdk8 branch 2 times, most recently from 354809c to 1ee6ace Compare February 26, 2021 23:31
@shakuzen
Copy link
Member

shakuzen commented Mar 1, 2021

I agree in general we should test we support what we say (building with JDK 8 in this case) or change what we say we support.
Our continuing to support building Micrometer with JDK 8 has been a convenience to contributors since JDK 8 is still the most common JDK. Also we haven't had a substantial reason to use Java 9+ features in our tests. It may be time to reconsider that decision. It does not seem overly burdensome to ask that contributors have JDK 11 (or even the latest JDK, probably) to build the project. The most important thing for users is that JDK 8 compatibility is maintained, which should be covered by the compiler release flag.

Of course this just shifts this issue from running a CI build with JDK 8 to JDK 11 if we bump the minimum JDK to build. I'm not sure the value is high enough to run such a job on every build - though the harm might be considered low also (more throttling from concurrent job limits). I wonder if a daily build to check the project can still be built with older JDK versions is sufficient, assuming we would notice a build failure of that daily job.

@shakuzen shakuzen mentioned this pull request Mar 1, 2021
@izeye izeye mentioned this pull request Aug 18, 2021
izeye added a commit to izeye/micrometer that referenced this pull request Aug 18, 2021
shakuzen pushed a commit that referenced this pull request Aug 20, 2021
@shakuzen
Copy link
Member

@izeye would you mind rebasing this now that the jdk11 build job is merged?

@izeye izeye changed the base branch from main to 1.5.x August 20, 2021 11:31
@izeye
Copy link
Contributor Author

izeye commented Aug 20, 2021

@shakuzen Sure, I've done that in 873e274.

@shakuzen shakuzen merged commit 0f78afe into micrometer-metrics:1.5.x Aug 23, 2021
@shakuzen shakuzen added build A change in our build-system type: task A general task labels Aug 23, 2021
@shakuzen shakuzen added this to the 1.6.11 milestone Aug 23, 2021
@izeye izeye deleted the jdk8 branch August 23, 2021 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build A change in our build-system type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants