Wrap types to prepare for the Any type #30
Conversation
andronat
requested review from
marioradonic,
paulquinn00,
matthewpaul,
ldulcic,
Michael-Preibisch and
karelispanagiotis
andronat
force-pushed
the
andronat/wrap-types
branch
from
508e7a2
to
b98ad3f
Compare
| @@ -112,7 +112,6 @@ message RelayPayload { | |||
| self_serve.orb.v1.AgeVerificationRequiredFromOperator age_verification_required_from_operator = 14; | |||
| self_serve.app.v1.StartCapture start_capture = 15; | |||
| self_serve.app.v1.RequestState request_state = 16; | |||
| self_serve.orb.v1.SelfServeStatus self_serve_status = 17; | |||
| self_serve.orb.v1.NoState no_state = 18; | |||
There was a problem hiding this comment.
Im a bit torn if the Any field should be inside the oneof at this moment. When we move fully to using the any_payload field we will want to remove the oneof completely and have the any_payload remain alone (is this agreeable to all?).
I believe doing this is not a breaking change so it may not be urgent, but there are side-effects: https://stackoverflow.com/questions/72746051/do-oneof-fields-show-up-on-the-wire-is-it-safe-to-move-a-field-out-of-a-oneof
One nit for hygiene is to number the any_payload field to be 100 (or any number outside the range used in the oneof) so we can cleanly reserve those old field numbers when the oneof goes away.
There was a problem hiding this comment.
I completely agree and I have further changes later on. But I wanted to break this down to smaller, easier to review steps. Here is how I envision the final message: 164632d
There was a problem hiding this comment.
Thanks for the preview. I like it!
There was a problem hiding this comment.
I also like the preview, that would be my choice also
| @@ -112,7 +112,6 @@ message RelayPayload { | |||
| self_serve.orb.v1.AgeVerificationRequiredFromOperator age_verification_required_from_operator = 14; | |||
| self_serve.app.v1.StartCapture start_capture = 15; | |||
| self_serve.app.v1.RequestState request_state = 16; | |||
| self_serve.orb.v1.SelfServeStatus self_serve_status = 17; | |||
| self_serve.orb.v1.NoState no_state = 18; | |||
There was a problem hiding this comment.
Thanks for the preview. I like it!
andronat
force-pushed
the
andronat/wrap-types
branch
from
b98ad3f
to
94a23b7
Compare
No description provided.