-
Notifications
You must be signed in to change notification settings - Fork 78
build: remove rules_proto dependency and update protobuf version #123
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
Conversation
dbd6c45
to
9117a26
Compare
b6ec17a
to
e4d6bcb
Compare
Signed-off-by: Matthieu MOREL <[email protected]>
c5bbad3
to
ae1eae9
Compare
Signed-off-by: Matthieu MOREL <[email protected]>
/cc @phlax , @adisuissa , I believe it's ready for your reviews. it's been tested on envoyproxy/envoy#41335 |
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.
Thanks, LGTM.
I'll let @phlax review as he has the most context around these changes.
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.
thanks @mmorel-35 - really appreciate you doing this work!!!
iiuc - we are adding rules_python etc because we previously pulled it in transitively from rules_proto?
wondering if we could/should break some of the changes out to separate PRs - but also not sure if that would make sense - so happy if we land what works
bazel/dependency_imports.bzl
Outdated
GO_VERSION = "1.24.6" | ||
|
||
# Python version for rules_python | ||
PYTHON_VERSION = "3.11" |
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.
envoy is using 3.12 - i would say go with that - ive held off 3.13 because imhe it breaks more stuff
…ports.bzl Signed-off-by: Matthieu MOREL <[email protected]>
@phlax , The addition of rules_protobuf is related to https://github.com/protocolbuffers/protobuf?tab=readme-ov-file#bazel-with-workspace I updated python version and updated the associated PR in envoy |
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.
got it, thanks for clarification - lgtm
Co-authored-by: phlax <[email protected]> Signed-off-by: Matthieu MOREL <[email protected]>
It's tested on envoy PR |
lgtm, but i dont have write powers in this repo - so @adisuissa will need to sign off |
Description
This is removing direct usage of rules_proto in xds. It also updates protobuf version and handle go dependencies.