GitHub actions cleanup #76
Conversation
Remove redundant test step from release workflow. Modify test workflow concurrency to cancel jobs that have a subsequent one queued to reduce load on the runners. Remove all permissions for the workflows by default. Change caching to only cache dependencies from the master branch. Still restore dependencies on PRs from the master branch cache.
| @@ -1,9 +1,12 @@ | |||
| name: Make release | |||
|
|
|||
| permissions: { } | |||
There was a problem hiding this comment.
Improve security by not using default permissions.
| java-version: 8 | ||
| distribution: 'temurin' | ||
| - name: Cache Maven packages | ||
| - name: Cache and restore Maven packages on master |
There was a problem hiding this comment.
The cache and restore followed by the restore on PR section is a bit redundant because this workflow only runs on the master branch. So, it should really only need this step without the if check. However, I think it's safer to keep it as is. Keeping it as is doesn't harm anything, it is a bit more complicated to read though.
| - name: Build | ||
| run: mvn clean verify | ||
|
|
||
| test: |
There was a problem hiding this comment.
mvn clean verify already runs tests. This step is redundant. The only difference is that install isn't executed anymore. I don't expect that to be an issue though, unless the publish step requires an install to happen first.
|
|
||
| - name: Publish package | ||
| run: mvn -DskipTests=true --batch-mode -P ossrh-publish -Dgpg.passphrase=${{ secrets.ORG_GPG_PASSPHRASE }} deploy | ||
| run: mvn -Dmaven.test.skip=true --batch-mode -P ossrh-publish -Dgpg.passphrase=${{ secrets.ORG_GPG_PASSPHRASE }} deploy |
There was a problem hiding this comment.
-Dmaven.test.skip=true will skip compiling and executing tests. -DskipTests=true only skips executing tests.
| - master | ||
| pull_request: | ||
|
|
||
| concurrency: |
There was a problem hiding this comment.
If the group matches, then cancel the existing workflow run and start a new one. Essentially, if you have a PR that is running a workflow and you push another change to it then the existing run will be cancelled and a new one started. This saves on some CI usage.
| java-version: 8 | ||
| distribution: 'temurin' | ||
| - name: Cache Maven packages | ||
| - name: Cache and restore Maven packages on master |
There was a problem hiding this comment.
PRs might contain various dependency changes that aren't always desired in the cache. So, only write to the cache from the master branch.
PRs will still restore from the master branch, so only dependency changes within that PR need to be downloaded.
| - master | ||
|
|
||
| jobs: | ||
| build: |
There was a problem hiding this comment.
Since you removed all permissions, you have to add the contents: read permission to this job.
There was a problem hiding this comment.
It seemed odd to me that I didn’t add any permissions back, but I didn’t see anything in the action I was referencing either.
I’ll check again and update. Same for the other file.
There was a problem hiding this comment.
Apperently, I was wrong. I always thought the contents: read permission was necessary for actions to get the repo content.
There was a problem hiding this comment.
I’m not really sure why, I’m just glad it works.
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| test: |
There was a problem hiding this comment.
This job also needs the contents: read permission.
Remove redundant test step from release workflow.
Modify test workflow concurrency to cancel jobs that have a subsequent one queued to reduce load on the runners.
Remove all permissions for the workflows by default.
Change caching to only cache dependencies from the master branch. Still restore dependencies on PRs from the master branch cache.