-
Notifications
You must be signed in to change notification settings - Fork 149
feat: add no-global-regex-flag-in-query
rule
#560
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
a82191b
to
89caf8c
Compare
no-global-regex-flag-in-query
rule
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.
@timdeschryver Thanks for implementing this so quickly! I added a couple of comments and asked for some small changes, but should be easy to address.
@Belco90 and @MichaelDeBoey thanks for quick review. |
I'd love to have the autofix, and should be fairly simple. However, I didn't want to request including this feature to merge this PR, so I'm happy about merging this one first and then add the autofix in a later PR if that's more convenient to you. |
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.
@Belco90 the last commit refactors the implementation with getDeepestIdentifierNode
. It does seem more robust to me, let me know what you think :)
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.
@timdeschryver Much better now, including the autofix function!
Just a couple of small tweaks and should be ready to go.
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 LGTM!
🎉 This PR is included in version 5.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Checks
Changes
no-global-regex-flag-in-query
Context
Resolves #559
It's been a long time since I've contributed, feel free to comment on this PR :)