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.

Sign up for GitHub

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

Jump to bottom

Upgrade to go1.21 #2033

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Upgrade to go1.21 #2033

merged 2 commits into from
Mar 5, 2024

Conversation

kiashok
Copy link
Contributor

@kiashok kiashok commented Feb 21, 2024 •
edited
Loading

Upgrade hcsshim/main to golang 1.21

@kiashok kiashok marked this pull request as ready for review February 21, 2024 23:43
@kiashok kiashok requested a review from a team as a code owner February 21, 2024 23:43
@kiashok
Copy link
Contributor Author

kiashok commented Feb 21, 2024

cc @helsaawy @anmaxvl

@kiashok
Copy link
Contributor Author

kiashok commented Feb 21, 2024

failures are not related

@kiashok
Copy link
Contributor Author

kiashok commented Feb 21, 2024

cc @akhilerm

akhilerm
akhilerm reviewed Feb 22, 2024
View reviewed changes
Copy link

@akhilerm akhilerm left a comment

Choose a reason for hiding this comment

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

Shouldn't the version be changed in the foilowing workflows also

@kiashok
Copy link
Contributor Author

kiashok commented Feb 22, 2024

Shouldn't the version be changed in the foilowing workflows also

made the changes

akhilerm
akhilerm approved these changes Feb 23, 2024
View reviewed changes
Copy link

@akhilerm akhilerm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

There are a few static check failures on unchanged files. But that can be taken up in a separate PR, or should we make the CI green before merge?

@kiashok
Copy link
Contributor Author

kiashok commented Feb 26, 2024

LGTM

There are a few static check failures on unchanged files. But that can be taken up in a separate PR, or should we make the CI green before merge?

I'd like to fix them before checking in the changes. Will look into them asap

@kiashok kiashok force-pushed the upgradeGo branch 3 times, most recently from 6e12ef7 to b0f4812 Compare February 27, 2024 04:35
@qmuntal
Copy link
Contributor

qmuntal commented Feb 27, 2024

FYI, Test_newBinaryCmd_Key_Value_Pair/Path_With_Back_Slashes tests probably due to this upstream change in filepath.Clean: golang/go@be9d78c.

My take is that the test expectation was incorrect, as that filepath.Clean presumably fixed some wrong UNC path processing.

ambarve
ambarve reviewed Mar 2, 2024
View reviewed changes
internal/cmd/io_npipe.go Outdated Show resolved Hide resolved
helsaawy
helsaawy approved these changes Mar 4, 2024
View reviewed changes
Copy link
Contributor

@helsaawy helsaawy left a comment

Choose a reason for hiding this comment

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

LGTM

kiashok and others added 2 commits March 4, 2024 14:35
@kiashok
Upgrade to go1.21 + fix lint errors
0f0812e 
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
@anmaxvl @kiashok
update newBinaryCmd URL path handling (#2041)
614dd02 
Signed-off-by: Maksim An <maksiman@microsoft.com>
@kiashok kiashok merged commit fe8c673 into main Mar 5, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anmaxvl anmaxvl anmaxvl approved these changes

@akhilerm akhilerm akhilerm approved these changes

@ambarve ambarve ambarve approved these changes

@helsaawy helsaawy helsaawy approved these changes

Assignees

@anmaxvl anmaxvl

@helsaawy helsaawy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kiashok @qmuntal @anmaxvl @akhilerm @ambarve @helsaawy