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
/.github/workflows/ci-check-repo.yaml: Iterating on github actions check repo #767
Conversation
dc56d8b
to
cdc1201
Compare
dbd197c
to
60fbb19
Compare
60fbb19
to
16a36eb
Compare
.github/workflows/ci-check-repo.yaml
Outdated
- name: Setup Go 1.13 | ||
uses: actions/setup-go@v2 | ||
with: | ||
go-version: 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.
must be 1.13
as ^1.13
resulted in a flag error for go get -mod=readonly ./...
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.
Can we upgrade the script to work with go > 1.13
? Maybe it's fine to just drop this statement in general. Or do GOFLAGS="-mod readonly" go build ./...
to check everything builds without modifications to go.mod
?
1dadad3
to
070e0f2
Compare
070e0f2
to
c3422e0
Compare
.github/workflows/ci-check-repo.yaml
Outdated
./utils/checkcommitters/check_pr.sh | ||
go vet -mod=readonly ./... | ||
go run -mod=readonly ./utils/copyrightshdrs/ | ||
go test -mod=readonly -test.v ./... |
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.
Is it necessary to run the tests here as well as in #762 ?
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.
No, we don't need to do this here.
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.
LGTM. Some comments.
.github/workflows/ci-check-repo.yaml
Outdated
./utils/checkcommitters/check_pr.sh | ||
go vet -mod=readonly ./... | ||
go run -mod=readonly ./utils/copyrightshdrs/ | ||
go test -mod=readonly -test.v ./... |
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.
No, we don't need to do this here.
fetch-depth: 0 | ||
- name: Check all | ||
working-directory: ./go | ||
# Keep this in sync with //go/utils/prepr/prepr.sh. |
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.
Can we update prepr.sh
to say "Keep this in sync with //.github/workflows/ci-check-repo.yaml
?
.github/workflows/ci-check-repo.yaml
Outdated
- name: Setup Go 1.13 | ||
uses: actions/setup-go@v2 | ||
with: | ||
go-version: 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.
Can we upgrade the script to work with go > 1.13
? Maybe it's fine to just drop this statement in general. Or do GOFLAGS="-mod readonly" go build ./...
to check everything builds without modifications to go.mod
?
looking into fixing this: https://github.com/liquidata-inc/dolt/pull/767/checks?check_run_id=808971285#step:4:296
the
exit status 128
is from//go/utils/checkcommiters/main.go
... im thinking could be the go version? i had to use1.13
here to get past the error that occurred when i used go version^1.13
regarding the-mod=readonly
flag....