-
Notifications
You must be signed in to change notification settings - Fork 602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BREAKING(yaml): rename ParseOptions.json
to ParseOptions.allowDuplicateKeys
#5282
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5282 +/- ##
=======================================
Coverage 95.76% 95.76%
=======================================
Files 457 457
Lines 37955 37955
Branches 5569 5569
=======================================
Hits 36346 36346
Misses 1568 1568
Partials 41 41 ☔ View full report in Codecov by Sentry. |
I'm in favor of this change. The option name I slightly feel |
I don't think it's any more natural than what it is now. |
I did a quick search - "duplicate keys" seems more correct. |
@@ -14,7 +14,7 @@ export interface ParseOptions { | |||
/** Name of the schema to use.*/ | |||
schema?: "core" | "default" | "failsafe" | "json" | "extended"; | |||
/** compatibility with JSON.parse behaviour. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also change the description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be reworking docs for all of @std/yaml
once the breaking changes are made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's changed
ParseOptions.json
has been renamedParseOptions.allowDuplicateKeys
. This change only affects those that previously setParseOptions.json
totrue
. No changes in behavior have been made.Motivation
This name more clearly describes how the option affects the behavior of
parse()
.Migration guide
To migrate, use the
allowDuplicateKeys
option instead of thejson
option forparse()
.Related
Towards #5195