Support duration-constant #5269
Conversation
pshao25
requested review from
m-nash,
lirenhe,
ArthurMa1978,
hallipr and
JoshLove-msft
as code owners
| @@ -51,7 +51,7 @@ public static InputDurationType CreateDurationType(ref Utf8JsonReader reader, st | |||
| crossLanguageDefinitionId = crossLanguageDefinitionId ?? throw new JsonException("Duration type must have crossLanguageDefinitionId"); | |||
| encode = encode ?? throw new JsonException("Duration type must have encode"); | |||
| wireType = wireType ?? throw new JsonException("Duration type must have wireType"); | |||
|
|
|||
| encode = encode == "duration-constant" ? "constant" : encode; | |||
There was a problem hiding this comment.
consider to combine this to L52
There was a problem hiding this comment.
we should not do it in this way, it would be hard to track these magic values.
Deserialization should just simply do "deserialization" without modification unless we could prove it is absolutely necessary
Therefore here we should deserialize this as is, and if other parts in our generator is expecting constant instead of duration-constant, those parts should change accordingly to properly handle duration-constant.
There was a problem hiding this comment.
I try to add something like TryReadEncoding, but that cannot keep throw new JsonException($"Encoding of Duration type {encode} is unknown.");. So changed by adding a CreateDurationKnownEncoding
Fix #5261