-
Notifications
You must be signed in to change notification settings - Fork 35
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
Adding git actions #20
Conversation
added GitActions to do the following: - get dependencies - check build - run tests - check format - check go mod status
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.
parser (binary file) got committed. Perhaps, we can create GiHub Actions to check for binary files.
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.
Seems parser
binary should be added to gitignore and removed from that PR
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.
Tested in #21
And seems works as expected:
- tests are disabled - job is finished /~https://github.com/devfile/parser/pull/21/checks?check_run_id=905590865
- tests failed - job failed /~https://github.com/devfile/parser/actions/runs/180806428
- go mod need to be updated /~https://github.com/devfile/parser/runs/905595388?check_suite_focus=true
- name: Run Go Tests | ||
run: go test ./... | ||
|
||
- name: Check format |
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.
It's not obvious but as far as I see now - it's better to check go.sum first and them go fmt
because go fmt
could also add missing dependencies to go.sum. See /~https://github.com/devfile/parser/runs/905595388?check_suite_focus=true
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.
+1 I ran into that as well.
|
The question: how that binary was built? If it's needed to manually type a command and specify binary name - anyone is free to name binary as they wish. |
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.
Approving it not to block PR merging but please address the following issues before merging:
- binary
- go dep
- go.sum and format checks orders
|
added GitActions to do the following:
Fixes #11, #17