Skip to content

[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

Merged
merged 8 commits into from
Oct 12, 2023
Merged

[cicd] Unify lint, add caching #1549

merged 8 commits into from
Oct 12, 2023

Conversation

mikeland73
Copy link
Contributor

Summary

  • unified gofumpt and lint
  • Added go caching to tests
  • Added pre-building devbox binary to be shared between tests

How was it tested?

CICD

@mikeland73 mikeland73 requested review from gcurtis and savil October 10, 2023 19:55
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
runs-on: ${{ matrix.os }}
Copy link
Collaborator

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?

Copy link
Contributor Author

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?

- name: Upload devbox artifact
uses: actions/upload-artifact@v3
with:
name: devbox-${{ runner.os }}
Copy link
Collaborator

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?

with:
args: "--out-${NO_FUTURE}format colored-line-number --timeout=10m"
skip-cache: true
- run: go run ./cmd/devbox run lint
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

- name: Build devbox
run: go install ./cmd/devbox
name: devbox-${{ runner.os }}
- run: chmod +x ./devbox
Copy link
Collaborator

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.

@mikeland73 mikeland73 requested a review from gcurtis October 10, 2023 21:31
@mikeland73
Copy link
Contributor Author

Comments addressed. @gcurtis PTAL

@mikeland73 mikeland73 merged commit 4380b7c into main Oct 12, 2023
@mikeland73 mikeland73 deleted the landau/unify-lint branch October 12, 2023 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants