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

Migrate CI from Travis to GitHub Actions #333

Merged
merged 2 commits into from
Nov 6, 2020
Merged

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Nov 6, 2020

Following the Travis OSS billing changes mentioned in #331, this PR migrates the Java build and checks to GH actions. It seems to work perfectly...

We won't run out of Travis minutes for quite a while, so I've left Travis enabled until we're convinced the new Actions agree with Travis.

GitHub shows an ugly diff given the file rename, the actual changes are all in 4b14f26.

Downsides: more eggs in GH basket, but Travis is basically forcing our hand...

@ml-evs ml-evs added priority/low There is a consensus that this have low priority to sort out. topic/infrastructure Issues/PRs relating to deployment and validation of the specification labels Nov 6, 2020
Copy link
Member

@merkys merkys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have insufficient knowledge of both Travis and GitHub Actions. Maybe someone having more experience here could review and approve?

@ml-evs ml-evs requested review from ltalirz and shyamd November 6, 2020 16:19
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @ml-evs !

Some slightly unrelated suggested changes. I've approved, since these suggested changes can be argued to be out-of-scope of this PR...

Comment on lines +27 to +28
run: |
pre-commit run --all-files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run: |
pre-commit run --all-files
run: pre-commit run --all-files

Comment on lines +24 to +25
run: |
pip install pre-commit==2.2.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
run: |
pip install pre-commit==2.2.0
run: pip install pre-commit==2.2.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should update this version to the latest version of 2.8.2?

Suggested change
run: |
pip install pre-commit==2.2.0
run: pip install pre-commit==2.8.2

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed this - should actually be java version 8, right? According to what is used currently in Travis?

.github/workflows/ci.yml Show resolved Hide resolved
@ml-evs
Copy link
Member Author

ml-evs commented Nov 6, 2020

Some slightly unrelated suggested changes. I've approved, since these suggested changes can be argued to be out-of-scope of this PR...

Agree with all your suggestions, but to save another round of reviewing I'll merge this as is. I'll raise another issue tracking the swagger validation of schema in the CI and can fix those minor blips then.

@ml-evs ml-evs merged commit dc980d3 into develop Nov 6, 2020
@CasperWA CasperWA deleted the ml-evs/ci_migration branch November 8, 2020 19:42
ml-evs added a commit that referenced this pull request Feb 16, 2021
* Added Java tests to GH actions
* Renamed GH actions file
ml-evs added a commit that referenced this pull request Feb 26, 2021
* Added Java tests to GH actions
* Renamed GH actions file
ml-evs added a commit that referenced this pull request May 30, 2021
* Added Java tests to GH actions
* Renamed GH actions file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/low There is a consensus that this have low priority to sort out. topic/infrastructure Issues/PRs relating to deployment and validation of the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants