-
Notifications
You must be signed in to change notification settings - Fork 80
feat: add allowLabels option to no-missing-label-refs #513
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
Conversation
|
Thanks for working on it. Just one thought: Would anyone be open to renaming the option to Since most rules in markdown use markdown/src/rules/no-duplicate-definitions.js Lines 70 to 71 in c790317
markdown/src/rules/no-empty-definitions.js Lines 89 to 90 in c790317
Line 70 in c790317
markdown/src/rules/no-unused-definitions.js Lines 71 to 72 in c790317
|
|
Yeah, that makes sense 👍. I went ahead and updated the option name. |
lumirlumir
left a comment
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.
Looks so good to me. Thanks 👍
Would like @nzakas to verify (including the option name) before merging.
|
@jaymarvelz please check the merge conflict. |
mdjermanovic
left a comment
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, thanks! Leaving open for @nzakas to verify, as requested.
nzakas
left a comment
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. Thanks!
Prerequisites checklist
What is the purpose of this pull request?
This PR adds a new
allowLabelsoption tono-missing-label-refs, allowing users to intentionally permit specific labels without definitions.What changes did you make? (Give an overview)
allowLabels.Related Issues
Fixes #449
Is there anything you'd like reviewers to focus on?