-
Notifications
You must be signed in to change notification settings - Fork 629
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
refactor(yaml): move duplicate
and duplicateIndex
#5836
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5836 +/- ##
==========================================
- Coverage 96.26% 96.25% -0.01%
==========================================
Files 479 479
Lines 38705 38698 -7
Branches 5617 5617
==========================================
- Hits 37258 37250 -8
Misses 1404 1404
- Partials 43 44 +1 ☔ View full report in Codecov by Sentry. |
} | ||
if (duplicate) { | ||
if (this.usedDuplicates.has(value)) return `*ref_${duplicateIndex}`; | ||
this.usedDuplicates.add(value); |
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.
This doesn't seem to pertain to the main objective of this PR.
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.
if (duplicate) {...}
would be checked twice otherwise, no?
if ( | ||
(tag !== null && tag !== "?") || | ||
duplicate || | ||
(this.indent !== 2 && level > 0) | ||
) { | ||
compact = false; | ||
} |
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.
Why was this moved?
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.
Because compact only needs to be evaluated after if (this.usedDuplicates.has(value)) return ``*ref_${duplicateIndex}``;
doesn't return early.
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.
Thank you
Changes
This PR moves
duplicate
andduplicateIndex
code insideisObject()
block, to avoid double check.Note One line
(tag !== null && tag !== "?")
is not covered. Seems like tag is always null if the value is an object.