-
Notifications
You must be signed in to change notification settings - Fork 372
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
test: add github action build test #502
test: add github action build test #502
Conversation
/assign @xing-yang |
LGTM Thanks @andyzhangx @jdef can you PTAL? CC @msau42 |
|
||
- name: Build Test | ||
run: | | ||
make -C lib/go protoc |
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.
make
make -C lib/go protoc
^ make
as a separate step/line, before installing deps, to run tests. which works as expected if you also ...
- update Makefile and lib/go/Makefile: s/TRAVIS/GITHUB_ACTIONS/
- drop the .travis.yml file
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.
fixed the comments, PTAL.
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 think this looks OK. Sad that we cannot run it before merging
Thank you both. |
Looks like we missed the build badge on the readme
…On Tue, Mar 29, 2022, 6:56 PM Saad Ali ***@***.***> wrote:
Merged #502 <#502>
into master.
—
Reply to this email directly, view it on GitHub
<#502 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAR5KLFNEA7234FJ7DR5TEDVCOKEBANCNFSM5RKGCSEQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
It seems that build test result won't show in the first github action PR, and it should show in the next PR after this PR merged.