-
Notifications
You must be signed in to change notification settings - Fork 779
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
feat(checks/label-content-name-mismatch): deprecate occuranceThreshold option in favor of occurrenceThreshold to fix typo #3782
Conversation
@@ -104,11 +104,11 @@ function shouldIgnoreHidden(virtualNode, context) { | |||
* @return {Boolean} | |||
*/ | |||
function shouldIgnoreIconLigature(virtualNode, context) { | |||
const { ignoreIconLigature, pixelThreshold, occuranceThreshold } = context; | |||
const { ignoreIconLigature, pixelThreshold, occurrenceThreshold } = context; |
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 should avoid a break in the API.
const { ignoreIconLigature, pixelThreshold, occurrenceThreshold } = context; | |
const { ignoreIconLigature, pixelThreshold } = context; | |
const occurrenceThreshold = context.occurrenceThreshold ?? context.occurenceThreshold |
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.
Thanks for this! I think you mean "occuranceThreshold" since that was the only version of the typo used in code. "Occurence" was only used in a comment. Added this modified fix in 22d9b95a1d851fae3c850f0a5f125a4ba8bf0438.
…lcoFiers' suggestions.
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.
Thanks, looking good so far, just need to update the name in one more place. Also, since we're going to deprecate the old name we'll need to update the pr title to match conventional commits. We deprecate things as a feat
and want to make sure the title says which thing is deprecated, and either the pr title or body should say what to use instead.
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.
Everything looks great. Looks like the tests are failing due to missing optional chaining. Tests should pass once it's added.
Reviewed for security |
Fixes typos of the word "occurrence" throughout axe-core.