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
Update: Add exceptionPatterns to id-length rule (fixes #13094) #13576
Conversation
Docs: Add "exceptionPatterns" to "id-length" rule
@sodaMelon Would you mind sign CLA ? |
Ok, I did it now. |
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 working on this 👍 I left some comments
I modified codes related with comments. Please check them whenever you have enough time. |
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!
Modification is complete. |
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 :)
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 working on this! Looks great, I have just a few small suggestions.
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
I modified the function name to While making this test, I thought "Z*" of the test case from the above test not look good. So I changed it to "^Z". Thanks for your 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.
LGTM, thanks!
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 for contributing!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[X] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
issue #113094
What changes did you make? (Give an overview)
I continued the PR #13099
Based on comments in #13099, I modified some codes about it.
Unnecessary 'parserOption' was removed from
id-length.js(test)
Two valid tests were added. They test with multiple exception patterns.
Two invalid tests were added, too. They test with an identifier that doesn't match configured pattern.
(One tests
tooLongError
, another teststooShrotError
. )To avoid creating new RegExp in "return value", I exatracted a function.
docs/rules/id-length.md
was updated aboutexceptionPatterns
Is there anything you'd like reviewers to focus on?
I want know how these test-cases are suitable.
I thought them in my own way.........🤔 but not sure.. 😂