Migrate from Travis CI to GitHub Actions #237
Conversation
There was a problem hiding this comment.
Looks good! 🚀 Thanks a lot for picking this up @ibabiankou 😄.
Added a commit. Feel free to change.
Already approving as the suggestions are not blocking :).
Suggested commit message:
Migrate from Travis CI to GitHub Actions (#237)
While there, drop the now-obsolete Takari Maven plugin.
pom.xml
Outdated
| <!-- We cap memory usage. This is especially relevant on Travis CI, | ||
| but locally this should also be more than enough. --> | ||
| -Xmx512m | ||
| <!-- We cap memory usage. This is especially relevant on CI, but |
There was a problem hiding this comment.
This change is also based on our internal build file.
There was a problem hiding this comment.
Hmm, we use a different text in error-prone-support, with the latter text seemingly of more recent vintage.
There was a problem hiding this comment.
Let's apply the same change as in PicnicSupermarket/error-prone-support#221. (I'll try to do a full review by EOD.)
README.md
Outdated
| [github-actions-build-badge]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml/badge.svg | ||
| [github-actions-build-master]: https://github.com/PicnicSupermarket/error-prone-support/actions/workflows/build.yaml?query=branch%3Amaster |
.github/workflows/build.yaml
Outdated
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
| push: | ||
| branches: [$default-branch] |
There was a problem hiding this comment.
We saw in PicnicSupermarket/error-prone-support#278 that unfortunately this doesn't work.
pom.xml
Outdated
| <!-- Our build system (Travis CI) provides a monotonically increasing | ||
| <!-- Our build system (GitHub Actions) provides a monotonically increasing | ||
| build number. When building locally, this number is obviously absent. | ||
| So we provide a default value. --> |
There was a problem hiding this comment.
I'll sync the phrasing with error-prone-support, but wonder: does this actually work still? (Something we can test separately; no need to block the PR over it.)
pom.xml
Outdated
| <!-- We cap memory usage. This is especially relevant on Travis CI, | ||
| but locally this should also be more than enough. --> | ||
| -Xmx512m | ||
| <!-- We cap memory usage. This is especially relevant on CI, but |
There was a problem hiding this comment.
Hmm, we use a different text in error-prone-support, with the latter text seemingly of more recent vintage.
.github/workflows/build.yaml
Outdated
| name: Build and verify | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
There are the default, so we can omit them.
.github/workflows/build.yaml
Outdated
| - name: Cache SonarCloud packages | ||
| if: ${{ matrix.sonarEnabled }} | ||
| uses: actions/[email protected] | ||
| with: | ||
| path: ~/.sonar/cache | ||
| key: ${{ runner.os }}-sonar | ||
| restore-keys: ${{ runner.os }}-sonar |
There was a problem hiding this comment.
Nice. (In a separate PR) we might want to do something similar for Maven dependencies.
(Or are those cached by default? Below we have "Remove installed project artifacts" after all.)
There was a problem hiding this comment.
Ah, nevermind; I now see the cache parameter to actions/setup-java.
.github/workflows/build.yaml
Outdated
| -Dsonar.host.url=https://sonarcloud.io | ||
| -Dsonar.organization=picnic-technologies | ||
| -Dsonar.projectKey=tech.picnic:oss-parent |
There was a problem hiding this comment.
The first two will also apply downstream; we can add them to pom.xml. The third should be inferred; let me test that 👀
|
Kudos, SonarCloud Quality Gate passed!
|
rickie
changed the title
Not sure if it's worth caching the SonarCloud packages, given there is only POM module here. 🤔