-
Notifications
You must be signed in to change notification settings - Fork 59
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
Move from Travis CI to Github Actions #103
Conversation
Signed-off-by: Akos Veres <[email protected]>
Signed-off-by: Akos Veres <[email protected]>
Signed-off-by: Akos Veres <[email protected]>
Signed-off-by: Akos Veres <[email protected]>
Signed-off-by: Akos Veres <[email protected]>
.github/workflows/build.yaml
Outdated
|
||
on: | ||
push: | ||
branches: [ master ] |
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.
Why is this limited to master?
.github/workflows/build.yaml
Outdated
build: | ||
strategy: | ||
matrix: | ||
go-version: [1.11.x] |
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.
Should be 1.13 now, thanks
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Signed-off-by: Akos Veres <[email protected]>
.github/workflows/publish.yaml
Outdated
publish: | ||
strategy: | ||
matrix: | ||
go-version: [1.11.x] |
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 should probably match the other workflows with 1.13
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.
Yes please
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.
Updated
Have you tested a release build on your own account? Most that I merged yesterday failed leaving broken tags |
Signed-off-by: Akos Veres <[email protected]>
I have as much as I could, in this repository we only take the git tag, not the sha and that looks correct. |
push: | ||
branches: "*" | ||
pull_request: | ||
branches: [ master ] |
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 should be "*" based on the discussions Slack
@akosveres do you have a moment to revisit the comments today? |
build: | ||
strategy: | ||
matrix: | ||
go-version: [1.13.x] |
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.
Do we need Go on the host?
This PR is required to complete our migration, so I'm closing due to inactivity. @utsavanand2 please can you take over in a new PR and make this work like the faas-netes or faas build please? Pay attention to how we set the version and commit information in the image and the GitHub Action. |
Description
The pull request has the following changes:
Motivation and Context
Fixes issue GitHub Actions Migration - tracking issue faas#1585
How Has This Been Tested?
I've done tests on my fork:
Successful build workflow https://github.com/akosveres/nats-queue-worker/runs/1365348972?check_suite_focus=true
Succesful publish workflow https://github.com/akosveres/nats-queue-worker/runs/1365361432?check_suite_focus=true
Types of changes
Checklist:
git commit -s