Skip to content

Remove the deprecated test for deprecated_disable_overprovisioning#17916

Merged
htuch merged 1 commit intoenvoyproxy:mainfrom
tyxia:overprovision
Sep 3, 2021
Merged

Remove the deprecated test for deprecated_disable_overprovisioning#17916
htuch merged 1 commit intoenvoyproxy:mainfrom
tyxia:overprovision

Conversation

@tyxia
Copy link
Copy Markdown
Member

@tyxia tyxia commented Aug 30, 2021

I plan to do the related clean-up work for illegal use of hidden_envoy_deprecated_ V2 check(in source/common/protobuf/utility.cc where this error message is thrown) in a separate PR.

Signed-off-by: Tianyu Xia tyxia@google.com

Risk Level: LOW
Testing: local unit test, CI

@tyxia
Copy link
Copy Markdown
Member Author

tyxia commented Aug 31, 2021

/assign @htuch

PTAL. Thanks!

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@htuch
Copy link
Copy Markdown
Member

htuch commented Sep 1, 2021

@tyxia before merging, can you explain why the split here? Generally prefer to group related changes unless there is some compelling reason.

@tyxia
Copy link
Copy Markdown
Member Author

tyxia commented Sep 2, 2021

@tyxia before merging, can you explain why the split here? Generally prefer to group related changes unless there is some compelling reason.

Hi @htuch , Thanks for the information. The reason I am doing clean-up in a separate PR #17924 is because 1) I feel this PR is just for disable_overprovisioning and the other one has board scope of change 2) The other one is riskier than this PR and I am working on fixing all the CI issue(it is still in draft mode :) )

But if merging them in one PR is preferred, please just let me know. I can definitely do it. Thanks!

@htuch
Copy link
Copy Markdown
Member

htuch commented Sep 3, 2021

Sure, let's just merge it.

@htuch htuch merged commit b59e0c0 into envoyproxy:main Sep 3, 2021
@tyxia tyxia deleted the overprovision branch September 8, 2021 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants