-
Notifications
You must be signed in to change notification settings - Fork 900
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
use single source for go version #4794
Conversation
Welcome @grosser! It looks like this is your first PR to karmada-io/karmada 🎉 |
Hi @grosser, do you have any documentation on how to use the |
I'm also curious about how it works. |
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.
You still need to using it on the workflow, otherwise it will not working.
- id: goversion
run: echo "goversion=$(cat .go-version)" >> "$GITHUB_OUTPUT"
- uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
with:
go-version: ${{ steps.goversion.outputs.goversion }}
trying with tools like go-env / asdf / mise can read this file to set the local go version |
I can see that the |
👋 need a CI approval |
The karmada's maintainers will return from vacation tomorrow. /lgtm |
I think it's need to squash the commit |
squashed into single commit if single commit is a requirement, making the PR template say it would speed up PRs |
and need another approval @liangyuanpeng |
I haven't used |
auto-switching switches the go version when cd-ing into a directory according to the directories .go-version |
I'm ok with either ... just want someone with merge permission to tell me what they would be willing to merge and will make adjustments accordingly. |
Thanks @grosser. This PR consists of two parts actually as you mentioned in the PR description:
Great thanks for bringing this discussion, really helpful!
For this part, I'm evaluating all these go-version managers(mise, asdf, go-env) to figure out the following questions:
|
It seems both
They both support |
This feature is awesome by the way. Just tried it out. |
why would we need .tool-versions, this repo only uses go right ? |
yeah, since I mean take |
I think @RainbowMango is mean we want to use There are two topic, the Part1 is go version for build & CI, Part2 is the file for people who using the go-version managers. I can see common practice ( kubernetes/kind/etcd ) is that But as @RainbowMango say, using
would you like to try with |
asdf also supports .go-version, so can we avoid 1 level of indirection by using that instead of .tool-versions ? |
Thanks for the reminder. I didn't notice it. then I belive |
By the way, the |
rebased + updated go.mod to have full version ... is this mergeable now or what still needs to change ? |
Signed-off-by: Michael Grosser <[email protected]>
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.
Looks good to me!
/lgtm
@liangyuanpeng Do you have further comments?
plz restart the 1 failed ci job, seems unrelated ... |
Done. |
still 2 checks pending ... any idea how to get them to pass ? |
All good now. Ready to move forward now. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Special notes for your reviewer:
Does this PR introduce a user-facing change?: