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

Reset source path of api.pb.go to pkg/api/api.proto #104

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

zxj874478410
Copy link
Contributor

Source path of api.pb.go is changed in #94 . This changes the variable naming in the file.
Tix this by regenerate api.proto.

@zxj874478410
Copy link
Contributor Author

It's my fault for changing the file generation path...pls @klihub @mikebrow

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

wave .. it happens.. :)

My bad, I didn't open the proto generated file to look over / review the diff..

Also the versioning for your build is off as set by the makefile helpers

https://github.com/containerd/nri/pull/94/files#diff-344492a28bbcb6eda633fc9255eaed8729b3076abd04bd05c4626bf25f7a10d6L18-R20

FYI these are the versions being used by containerd .. protobuf version v3.20.1 and protoc v1.28.0

https://github.com/containerd/nri/blob/main/scripts/install-protobuf#L23
https://github.com/containerd/nri/blob/main/Makefile#L188

@mikebrow mikebrow added this to the 1.0 milestone Aug 20, 2024
@zxj874478410
Copy link
Contributor Author

wave .. it happens.. :)

My bad, I didn't open the proto generated file to look over / review the diff..

Also the versioning for your build is off as set by the makefile helpers

https://github.com/containerd/nri/pull/94/files#diff-344492a28bbcb6eda633fc9255eaed8729b3076abd04bd05c4626bf25f7a10d6L18-R20

FYI these are the versions being used by containerd .. protobuf version v3.20.1 and protoc v1.28.0

https://github.com/containerd/nri/blob/main/scripts/install-protobuf#L23 https://github.com/containerd/nri/blob/main/Makefile#L188

Fixed withprotobuf version v3.20.1 and protoc v1.28.0 :)

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@klihub
Copy link
Member

klihub commented Aug 22, 2024

It's my fault for changing the file generation path...pls @klihub @mikebrow

@zxj874478410 Normally, you should not need to do manually proto-compile anything. The Makefile has the necessary dependencies and targets to automatically proto-compile if the proto-file changes. The plugins don't get recompiled in that case without a make clean, but the compiled proto files do...

@klihub klihub self-requested a review August 22, 2024 14:51
Copy link
Member

@klihub klihub left a comment

Choose a reason for hiding this comment

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

LGTM

@mikebrow mikebrow merged commit e1d8260 into containerd:main Aug 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants