Skip to content

Remove github.com/gogo/protobuf again#7825

Merged
fuweid merged 2 commits intocontainerd:mainfrom
kzys:no-gogo
Dec 16, 2022
Merged

Remove github.com/gogo/protobuf again#7825
fuweid merged 2 commits intocontainerd:mainfrom
kzys:no-gogo

Conversation

@kzys
Copy link
Copy Markdown
Member

@kzys kzys commented Dec 15, 2022

While we need to support CRI v1alpha2, the implementation doesn't have to be tied to gogo/protobuf.

Signed-off-by: Kazuyoshi Kato [email protected]

@k8s-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kzys kzys force-pushed the no-gogo branch 4 times, most recently from 84e563f to 9b672c4 Compare December 15, 2022 18:57
@kzys kzys marked this pull request as ready for review December 15, 2022 19:04
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM, but one nit about indirect deps in the "main" clause of go.mod

Comment thread go.mod Outdated
github.com/emicklei/go-restful/v3 v3.8.0
github.com/fsnotify/fsnotify v1.6.0
github.com/gogo/protobuf v1.3.2
github.com/gogo/protobuf v1.3.2 // indirect
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit, we usually move indirect deps in the next "indirect" collection to make it clearer

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm maybe my Go toolchain is old? Let me check.

Kazuyoshi Kato added 2 commits December 15, 2022 22:54
While we need to support CRI v1alpha2, the implementation doesn't have
to be tied to gogo/protobuf.

Signed-off-by: Kazuyoshi Kato <[email protected]>
@fuweid fuweid merged commit 5ef7ea4 into containerd:main Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants