[WIP] Validate deprecated fields via validation visitor#10307
[WIP] Validate deprecated fields via validation visitor#10307nezdolik wants to merge 3 commits intoenvoyproxy:masterfrom
Conversation
a3f1d90 to
39eb22b
Compare
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
39eb22b to
6158514
Compare
htuch
left a comment
There was a problem hiding this comment.
@nezdolik The bits that you have here so far look good, thanks for taking this on, but I think at the call site in source/common/protobuf/utility.ccwe're going to expect similar behavior no matter what validation visitor was plumbed through for unknown fields, i.e. exception behavior.
So, I think it's fine to factor the body of this basic block: https://github.com/envoyproxy/envoy/blob/master/source/common/protobuf/utility.cc#L200
into a utility and call from onDeprecated in both warn/strict validation visitors (that site is also where you would call into onDeprecatedField).
Happy to chat more on Slack, this comes down to the qualitative difference between deprecated and unknown fields in how we do breaking changes.
Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com>
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
still wip |
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Signed-off-by: Kateryna Nezdolii nezdolik@spotify.com
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Description: TBD
Risk Level: TBD
Testing: TBD
Docs Changes: TBD
Release Notes: TBD
Fixes #8092