-
Notifications
You must be signed in to change notification settings - Fork 37
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
ci: use docker-generate-protobuf by default in Makefile #663
base: main
Are you sure you want to change the base?
Conversation
We need to update protobuf versions inside the tools container as well. |
This commit modifies Makefile to use docker-generate-protobuf instead of generate-protobuf by default. Signed-off-by: Rakshith R <[email protected]>
99a9fbd
to
a5154fd
Compare
where exactly ? |
@@ -1,7 +1,7 @@ | |||
// Code generated by protoc-gen-go. DO NOT EDIT. | |||
// versions: | |||
// protoc-gen-go v1.34.2 | |||
// protoc v3.20.2 | |||
// protoc v3.19.6 |
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.
We should keep this to the same version as we have in GitHub action
protobuf-compiler \ |
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'd prefer to remove the protoc installation in the github action. The CI job calls make test
, so it should run in the container now too.
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.
Once this is addressed, it should be good to go.
Right now the tools container uses the latest available version from Fedora. We should pin the version to that in the spec repo. We could have something like this for protoc tools. |
@@ -1,7 +1,7 @@ | |||
// Code generated by protoc-gen-go. DO NOT EDIT. | |||
// versions: | |||
// protoc-gen-go v1.34.2 | |||
// protoc v3.20.2 | |||
// protoc v3.19.6 |
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.
Once this is addressed, it should be good to go.
This commit modifies Makefile to use docker-generate-protobuf instead of generate-protobuf by default.