-
Notifications
You must be signed in to change notification settings - Fork 249
[cicd] Unify lint, add caching #1549
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
Conversation
strategy: | ||
matrix: | ||
os: [ubuntu-latest, macos-latest] | ||
runs-on: ${{ matrix.os }} |
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.
Would it be faster to build all platforms on a single machine and set GOOS/GOARCH?
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.
That would be a bit slower no? In this case they can run in parallel?
.github/workflows/cli-tests.yaml
Outdated
- name: Upload devbox artifact | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: devbox-${{ runner.os }} |
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.
Could we throw in runner.arch
too just in case we start using M1/M2 runners?
.github/workflows/cli-tests.yaml
Outdated
with: | ||
args: "--out-${NO_FUTURE}format colored-line-number --timeout=10m" | ||
skip-cache: true | ||
- run: go run ./cmd/devbox run lint |
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.
Use the built artifact here too?
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 changed this, but I think longer term we can use the released devbox instead for speed since this is one of the first tests we want to see complete.
.github/workflows/cli-tests.yaml
Outdated
- name: Build devbox | ||
run: go install ./cmd/devbox | ||
name: devbox-${{ runner.os }} | ||
- run: chmod +x ./devbox |
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 it also needs to be added to the PATH.
Comments addressed. @gcurtis PTAL |
Summary
How was it tested?
CICD