-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
[actions] Add CI for jdk 8, 11, 17, and 18-ea #254
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
name: Java CI | ||
|
||
on: [push, pull_request] | ||
|
||
jobs: | ||
test: | ||
runs-on: ${{ matrix.os }} | ||
strategy: | ||
matrix: | ||
os: [ubuntu-latest, macOS-latest, windows-latest] | ||
java: [8, 11, 17, 18-ea] | ||
distribution: ['zulu'] | ||
fail-fast: false | ||
max-parallel: 4 | ||
name: Test JDK ${{ matrix.java }}, ${{ matrix.os }} | ||
|
||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Set up JDK | ||
uses: actions/setup-java@v2 | ||
with: | ||
java-version: ${{ matrix.java }} | ||
distribution: ${{ matrix.distribution }} | ||
- name: Test with Maven | ||
run: ./mvnw test -B -D"license.skip=true" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend adding
workflow_dispatch
to allow manual triggering, and limiting pushes tomaster
branch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If i get that one properly, it would not normally auto run on other branches (say on a fork with many as I just did to raise a bunch of PRs - but would on pull request for sure) unless going into the workflow_dispatch and triggering said branch. I had not seen that one but looked on oshi and sort of understand that and just wanted to clarify my assumptions. Assuming I have that correct, sounds like a good idea as most contributors will never have anything but master anyways ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.
As I wrote it above the branch restriction only applies to "push" to the repository, so doesn't stop any PR runs.
I think it's always a good idea to add
workflow_dispatch
.The master branch restriction may not be relevant here, but there is a
generate-site
branch that can be pushed to, and there's no need to run actions when that happens. Addingworkflow_dispatch
allows you to manually choose to run it on any other branches if needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch cannot be used: this is a temporary branch that is automatically generated by a GA after a PR is merged in order to generate the snapshot doc.
Any change to the master branch will be detected by Gihub and it will re-generate the Jekyll website.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this PR has been merged, but I did want to point out that any branch, temporary or otherwise, generated by anyone (human or automation) qualifies as a "push" to the repository and would trigger a build. It's not a big deal if it's only done rarely (and it's free anyway).
Speaking of free and to close the loop on billing, GHA is free with no quota for public repos like this one. I migrated my project over two years ago (well, @hazendaz did!) and have run 3400 workflows since then, most in the 5 minute range, some taking hours. I haven't paid a dime. GHA is owned by Microsoft and backed by the massive Azure cloud, so resource limits and backlogs aren't really a thing.
Travis claims to be free but allocates minutes to OSS projects that must be applied for, but more importantly they have a limited number of machines and I honestly don't know how long they'll be around. I still use Travis only for an ARM hardware build because GHA doesn't have those (yet... guess what one of the things I'm supporting at work is) and ARM provides those machines for free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbwiddis : I've just merged this PR and did the updates in a subsequent commit after ;-) You can have a look at the code in master now. It's clean IMO. Let me know 👍