AGTMETRICS-340: Migrate CI from CircleCI to GitHub Actions#309
Conversation
Replaces .circleci/config.yml with .github/workflows/ci.yml, preserving the same test matrix: Java 7/8/11/13/17 on Linux, Java 12 on Windows, and dedicated jobs for jnr-exclude and jnr-latest profiles. Adds Maven dependency caching as a free improvement over the previous setup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6258ca7a6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest] | ||
| java-version: ['7', '8', '11', '13', '17'] |
There was a problem hiding this comment.
Including java-version: '7' in this matrix will make the new GitHub Actions job fail before tests run, because ubuntu-latest runners currently ship Maven 3.9.x while Maven 3.9 requires JDK 8+ to execute. This workflow invokes plain mvn for every matrix entry, so the Java 7 leg is effectively broken unless you install/pin an older Maven (or use a wrapper) specifically for that leg.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep producing 1.7 artifacts, but use a more modern compiler to do so. We used to rely on source/target flags to support an earlier version of Java, but had to run 1.7 in CI to verify we didn't accidentally use newer APIs. While Java 1.7 is available in GH actions, maven doesn't quite work. With Java 9 we can use release flag instead of source/target, which also checks that we only use classes compatible with 1.7 classpath, so we don't have to run maven 1.7 in CI.
StephenWakely
left a comment
StephenWakely
left a comment
There was a problem hiding this comment.
I think the PR description is wrong:
Preserves the same test matrix: Java 7, 8, 11, 13, 17
We've dropped 7 and 8.
Also wondering about this comment in the readme:
The library supports Java 1.7+.
Is this completely accurate? We don't test on 1.7 anymore, but produce artifacts that should theoretically work. Is that something we could clarify better?
| include: | ||
| - os: windows-latest | ||
| java-version: '12' |
There was a problem hiding this comment.
Is there a reason we are testing only 12 on Windows?
|
@StephenWakely Good points, I updated the description.
I believe this is still accurate. Both source code and released artifacts should still be fully compatible, and release flag ensures that. Only our build system isn't, but that hardly changes anything for the users. |
3 participants
Summary
.circleci/config.ymlwith.github/workflows/ci.ymlPreserves the same test matrix: Java7, 8,9, 11, 13, 17 on Linux; Java 12 on Windows; dedicated jobs forjnr-excludeandjnr-latestprofilesactions/setup-java(free improvement — CircleCI had no caching)Uses Zulu JDK distribution for broadest version coverage (Java 7+), replacing the third-partyjfullaondo/openjdk:7Docker image--releaseflag to continue targeting 1.7 with released artifacts).Test plan
CIworkflow appears under the Actions tab after pushingtest-jnr-exclude+test-jnr-latestdogstatsd-http-corebuild🤖 Generated with Claude Code