Skip to content

Conversation

hzxuzhonghu
Copy link
Member

@hzxuzhonghu hzxuzhonghu commented Apr 24, 2022

Please provide a description of this PR:

Looks we do not need them anymore. And there is no automatically sync with common-files repo

@hzxuzhonghu hzxuzhonghu requested a review from a team as a code owner April 24, 2022 08:49
@istio-policy-bot istio-policy-bot added the release-notes-none Indicates a PR that does not require release notes. label Apr 24, 2022
@istio-testing istio-testing added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 24, 2022
@hzxuzhonghu hzxuzhonghu requested a review from a team as a code owner April 24, 2022 09:23
Copy link
Member

@howardjohn howardjohn left a 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:
Copy link
Member

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

Copy link
Member Author

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

Copy link
Member

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"
Copy link
Member

Choose a reason for hiding this comment

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

why is this changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

make gen does

Copy link
Member

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"
Copy link
Member

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

@istio-testing istio-testing merged commit 6940b6a into istio:master Apr 25, 2022
@ericvn
Copy link
Contributor

ericvn commented Apr 25, 2022

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.

@hzxuzhonghu hzxuzhonghu deleted the rm-common-protos branch April 26, 2022 01:20
GregHanson pushed a commit to GregHanson/istio that referenced this pull request May 13, 2022
* Remove common-protos

* Make gen
istio-testing pushed a commit that referenced this pull request May 13, 2022
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes-none Indicates a PR that does not require release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants