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

Revert "use go v1.18 and update dependencies" #528

Merged

Conversation

humblec
Copy link
Contributor

@humblec humblec commented Oct 27, 2022

Additional Info#
This reverts commit 150c9d9 partially.
It keep the go.mod module version to 1.18 but leave old versions of genproto.
Dropping this dependency back to its prior level would help downstreams avoid pulling in a ton of extra deps till we figure out how to consume bits from genproto in a more isolated way

Ref# 150c9d9#r87967287

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce an API-breaking change?:

none

@humblec
Copy link
Contributor Author

humblec commented Oct 27, 2022

Cc @liggitt

@bertinatto
Copy link

/assign @saad-ali @jdef
Who LGTM'd the previous PR.

@liggitt
Copy link
Contributor

liggitt commented Oct 27, 2022

The go1.18 change was fine, the genproto bump was the issue

@bertinatto
Copy link

Also, note that v1.5.2 is not the latest version of protobuf. The module has been renamed and migrated to https://github.com/protocolbuffers/protobuf-go.

This reverts commit 150c9d9 partially.
It keep the go.mod module version to 1.18 but leave old versions of
genproto.
Copy link
Member

@jdef jdef left a comment

Choose a reason for hiding this comment

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

LGTM

@jdef
Copy link
Member

jdef commented Oct 28, 2022

@saad-ali it seems this would be worth cherry-picking for 1.7

@saad-ali
Copy link
Member

LGTM

Will merge and cherry pick to 1.7

@saad-ali
Copy link
Member

Is there a related bug or issue where this was discussed? Curious who/where the related issues were caught?

Copy link
Member

@saad-ali saad-ali left a comment

Choose a reason for hiding this comment

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

LGTM

@saad-ali saad-ali merged commit 9358408 into container-storage-interface:master Oct 31, 2022
saad-ali added a commit that referenced this pull request Nov 1, 2022
@humblec
Copy link
Contributor Author

humblec commented Nov 2, 2022

Is there a related bug or issue where this was discussed? Curious who/where the related issues were caught?

@saad-ali The references I have here is the one in PR description (150c9d9#r87967287) from @liggitt

@liggitt
Copy link
Contributor

liggitt commented Nov 2, 2022

See kubernetes/kubernetes#113366

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.

5 participants