Skip to content

remove support of hidden_deprecated_use_alpha#17169

Merged
htuch merged 4 commits intoenvoyproxy:mainfrom
ankatare:deprecated_use_alpha
Jul 21, 2021
Merged

remove support of hidden_deprecated_use_alpha#17169
htuch merged 4 commits intoenvoyproxy:mainfrom
ankatare:deprecated_use_alpha

Conversation

@ankatare
Copy link
Copy Markdown
Contributor

Signed-off-by: Abhay Narayan Katare abhay.katare@india.nec.com

Commit Message: Another PR of type remove_v2_support. first PR is #16274
Additional Description: Removing support of hidden_deprecated_use_alpha
Risk Level: LOW
Testing: unit (i.e. bazel test under bazel test test/extensions/filters/http/ext_authz/... )
Docs Changes: NA
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@ankatare
Copy link
Copy Markdown
Contributor Author

@htuch PTAL

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Jun 29, 2021

Thanks for working on this! unsolicited reviews here :)
Maybe we can consider to clean up the code base a bit further in this PR. I just did a quick search for keyword use_alpha
Just a few examples:

  1. get rid of use_alpha default arg from function getVersionedMethodPath, getVersionedServiceFullName() in file envoy/test/test_common/utility.h and related caller sites (e.g. envoy/test/integration/hds_integration_test.cc)
  2. get rid of default arg use_alpha from function const Protobuf::MethodDescriptor& getMethodDescriptorForVersion(..)

@ankatare
Copy link
Copy Markdown
Contributor Author

@tyxia Thanks for review it.
i think Yes. ideally code related to v2/v2alpha should also be removed , however looking comments at #16274
i specifically looking at keywords "hidden_envoy_deprecated_*"

@htuch please suggest.

@htuch
Copy link
Copy Markdown
Member

htuch commented Jun 30, 2021

Yeah, I think in the case of ext_authz, the *use_alpha code is truly dead and we can delete it everywhere.

@ankatare
Copy link
Copy Markdown
Contributor Author

@htuch Ok. Thanks. Will get back ASAP

@htuch htuch added the waiting label Jul 1, 2021
@ankatare
Copy link
Copy Markdown
Contributor Author

@htuch sorry for delay due to bad health :( . i will push changes ASAP

Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
@ankatare ankatare force-pushed the deprecated_use_alpha branch from c7fb819 to 796c932 Compare July 19, 2021 10:36
…from code

Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
@ankatare ankatare force-pushed the deprecated_use_alpha branch from 796c932 to d1172bb Compare July 19, 2021 10:37
@ankatare
Copy link
Copy Markdown
Contributor Author

Sorry, seems some format issue :( . push the changes ASAP

ankatare added 2 commits July 19, 2021 20:01
Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
…entSharedPtr

Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
@ankatare
Copy link
Copy Markdown
Contributor Author

@htuch please review

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 htuch merged commit 47160ca into envoyproxy:main Jul 21, 2021
@ankatare ankatare deleted the deprecated_use_alpha branch July 21, 2021 05:20
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Removing support of hidden_deprecated_use_alpha

Risk Level: LOW
Testing: unit (i.e. bazel test under bazel test test/extensions/filters/http/ext_authz/... )

Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
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.

3 participants