Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[3.4] Fix all the pipeline failues for release 3.4 #14136
[3.4] Fix all the pipeline failues for release 3.4 #14136
Changes from all commits
1abf085
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is it intended to turn
RACE
of permanently ?I see you fix some of the races in the other places in this PR.
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 just keep it as it's. Previously when go version is 1.15.15, then
RACE
is set tofalse
, see tests.yaml#L47There 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 we should have at least one test running with
RACE='true'
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.
Yes, I agree. But let's add any new pipeline/test in a separate PR. I just bumped the go version to 1.16.15 in this PR, and keep all others unchanged.
Previously when go version is 1.15.15, then RACE is set to false, see tests.yaml#L47. So I just keep it as it's.
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 the idea was to keep this at 1.12 #12840 (comment) CC @ptabor
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.
No need to support go 1.12. It's out of support, and is subject to vulnerability. IIRC It is also not compatible with some system packages in newer version.
Note that it's more than 1 year before when ptabor added that comment, and go 1.12 wasn't that old although it's also out of support at that time.
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'm not 100% sure but I think go 1.16 keeps go compatibility promise, so its not subject to vulnerabilities.
I also run into package issues but
go mod tidy
fixed it ff89836Either way is fine with me, but both will work IMO.
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.
Note that the promise of compatibility is for golang language of version < 2.0. There is no guarantee on the core library and tool-chain.
This file was deleted.