refactor: JsonSchema remove preserveSelfUnknownFields #4425
Conversation
| if (p.isPreserveSelfUnknownFields()) { | ||
| preserveSelfUnknownFields = true; | ||
| } | ||
| preserveUnknownFields = p.isPreserveUnknownFields(); |
There was a problem hiding this comment.
If I remember correctly this method can be called multiple times on objects, so that the last property that affect the object may reset the flag, the gut feeling is to leave the if:
if (p.isPreserveUnknownFields()) {
preserveUnknownFields = p.isPreserveUnknownFields();
}but haven't checked the code carefully.
There was a problem hiding this comment.
I just removed the if-else clause because it seemed redundant, but what you say is true. I'll revert this change
There was a problem hiding this comment.
Looking at it again, and the same comment might apply to the other properties as well (min, max, pattern). 👍
There was a problem hiding this comment.
You're right. I changed them, please check again 🙏
There was a problem hiding this comment.
This encoding works for me 👍
I'm negatively surprised that the test suite is not able to catch a regression here 🙁
There was a problem hiding this comment.
A next step would be to implement tests that verify the described expected behaviors (multipass processing, etc.)
Signed-off-by: Marc Nuri <marc@marcnuri.com>
manusa
force-pushed
the
refactor/preserve-unknown-fields
branch
from
3b45d8c
to
dbb6163
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Description
refactor: JsonSchema remove preserveSelfUnknownFields
relates to: #4398
Type of change
test, version modification, documentation, etc.)
Checklist