-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Remove common-protos #38546
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
Remove common-protos #38546
Conversation
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.
lgtm
@@ -107,13 +104,6 @@ update-common: | |||
@cp -a $(TMP)/common-files/files/* $(shell pwd) | |||
@rm -fr $(TMP)/common-files | |||
|
|||
update-common-protos: |
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.
do not change this, this file is synced with common-files
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 donot know why the proto is used in main repo. Can you clarify
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.
Makefile.common.mk does many many things, one of them is update-common-protos.
If you make this change, when update-common
is ran (which is done automatically) it will revert this change anyways
protoreflect "google.golang.org/protobuf/reflect/protoreflect" | ||
protoimpl "google.golang.org/protobuf/runtime/protoimpl" | ||
_ "google.golang.org/protobuf/types/known/anypb" |
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.
why is this changed?
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.
make gen does
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.
ah I get it now, nvm. lgtm
protoreflect "google.golang.org/protobuf/reflect/protoreflect" | ||
protoimpl "google.golang.org/protobuf/runtime/protoimpl" | ||
_ "google.golang.org/protobuf/types/known/anypb" |
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.
ah I get it now, nvm. lgtm
Note that the common files were set back to their prior values when the next common-files update happened. This changes need to be done upstream in common-files. |
* Remove common-protos * Make gen
* operator: update golden files (#38714) Currently operator tests use a snapshot of the manifests for tests to avoid churn. Update these to meet current state of the world. All changes are autogenerated * update autoscaling version to v2beta2 (#37872) * update autoscaling version to v2beta2 * integrate with api repo changes * address comments * update helm gateway * address comments, fix boolean condition * modify pruning based on k8s version * fix comments * bump pdb version to policy/v1 (#38814) * add v1 pdb * release note * Remove common-protos (#38546) * Remove common-protos * Make gen Co-authored-by: John Howard <howardjohn@google.com> Co-authored-by: Xinnan Wen <iamwen@google.com> Co-authored-by: Zhonghu Xu <xuzhonghu@huawei.com>
Please provide a description of this PR:
Looks we do not need them anymore. And there is no automatically sync with common-files repo