Skip to content
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

Run CI on all supported versions of Go #356

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucacome
Copy link
Contributor

This was initially part of #354 but I decided to create a separate PR, so it can be maybe merged sooner.

@lucacome
Copy link
Contributor Author

lucacome commented Oct 7, 2024

I didn't remove the hermit directory because I'm not sure if it's used for local development as well, I'm not familiar with the tool.

run: ./bin/hermit env -r >> $GITHUB_ENV

- name: Setup Golang Environment
uses: actions/setup-go@v5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, It's recommended to pin this to a git hash for supply chain security.

Renovate can also keep this up to date.

Suggested change
uses: actions/setup-go@v5
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but this repo is not really maintained...I'm not sure who would merge all the PRs...

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 mean I’d be down to help maintain it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the effort but I personally don't think it's useful to test on historical versions of Go at all, for multiple reasons:

  1. Only the most recent 2 versions are supported by the Go team, and I generally adhere to this policy too.
  2. The Go backwards compatibility promise is incredibly reliable.

The other changes seem reasonable.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My usual recommendation here is to update the go directive in the go.mod file to support the latest 2 releases. This way the matrix test matches. This is the policy we use for most of the Prometheus ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants