-
Notifications
You must be signed in to change notification settings - Fork 160
add mockgen check for CI #720
Conversation
qdm12
left a comment
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.
Great, I was about to do it after our little mock conversation 😄 Thanks!
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-go@v4 |
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.
We can use v5 😉
| - uses: actions/setup-go@v4 | |
| - uses: actions/setup-go@v5 |
| - uses: actions/setup-go@v4 | ||
| with: | ||
| go-version: ${{ env.min_go_version }} | ||
| check-latest: 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.
So we use the Go version specified and not the latest
| check-latest: true |
| - shell: bash | ||
| run: scripts/mock.gen.sh | ||
| - shell: bash | ||
| run: .github/workflows/check-clean-branch.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.
Why not directly inline the script content in there + remove .github/workflows/check-clean-branch.sh (one less file is always a win 😄)
| run: .github/workflows/check-clean-branch.sh | |
| run: | | |
| git update-index --really-refresh >> /dev/null | |
| git diff --exit-code |
Also do we really need git update-index --really-refresh >> /dev/null? I never had the use case, I think git diff --exit-code should suffice to find diffs and exit with 1 if any diff is found.
EDIT: Happy to do the "sibling PR" in subnet-evm to have the same thing.
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 just copied this from avalanchego repo which has this check. Not sure that it's needed. IMO we can close this PR and handle it in #721 (comment)
|
Bundled within #721 so closing this 😉 |
Copying this check from subnet-evm to ensure mocks are up to date.