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

[chore] Use check-latest in actions/setup-go #4617

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Oct 12, 2023

Avoid problems like this one #4607 by using check-latest-version

Opportunistic improvements:

@pellared
Copy link
Member Author

pellared commented Oct 12, 2023

When check-latest: true is used we can change the go-version values to "1.20" and "1.21". Can I do this (upvote) or do you prefer to still keep the explicit minimal patch version (downvote)?

@pellared
Copy link
Member Author

When check-latest: true is used we can change the go-version values to "1.20" and "1.21". Can I do this (upvote) or do you prefer to still keep the explicit minimal patch version (downvote)?

Done aa526b7

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@pellared
Copy link
Member Author

pellared commented Oct 13, 2023

Now the lint job is ~1 min faster 💪 (maybe it was just luck, but I think it should be in average faster)

main 5m 14s
PR 3m 54s

@MrAlias
Copy link
Contributor

MrAlias commented Oct 13, 2023

When check-latest: true is used we can change the go-version values to "1.20" and "1.21". Can I do this (upvote) or do you prefer to still keep the explicit minimal patch version (downvote)?

I would rather keep the minimums pegged above versions with CVEs. This will help ensure no issues are re-introduced.

@pellared
Copy link
Member Author

When check-latest: true is used we can change the go-version values to "1.20" and "1.21". Can I do this (upvote) or do you prefer to still keep the explicit minimal patch version (downvote)?

I would rather keep the minimums pegged above versions with CVEs. This will help ensure no issues are re-introduced.

@MrAlias, what do you mean by re-introducing? A regression bug that we remove check-latest: true? If this would happen we would simply add it back.

Is your intention that we bump the version each time a Go patch is released? If so then why maybe we should be pinning to the Go version instead e.g. `go-version: "1.21.3"? This would give the advantage of having more reproducible builds.

@pellared
Copy link
Member Author

pellared commented Oct 16, 2023

Merging as at least the CI is quicker. I created #4639 for potentially addressing #4617 (comment).

@pellared pellared merged commit 9900450 into open-telemetry:main Oct 16, 2023
24 checks passed
@pellared pellared deleted the go-check-lastest branch October 16, 2023 09:22
@MrAlias
Copy link
Contributor

MrAlias commented Oct 16, 2023

I would have appreciated it if all feedback was addressed prior to this merging, in accordance with our policy.

@pellared
Copy link
Member Author

I would have appreciated it if all feedback was addressed prior to this merging, in accordance with our policy.

  1. It was not clear that it isa blocker feedback (there was no "request changes").
  2. There was no consensus as @MadVikingGod upvoted my proposal.

I found it more beneficial to merge as it is and created an issue as part of addressing the feedback. I had no bad intentions 😉

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.

4 participants