-
-
Notifications
You must be signed in to change notification settings - Fork 938
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
Fix false positives for interpolation in property name in custom-property-pattern #5949
Conversation
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.
@kawaguchi1102 Thanks for jumping on this.
Changes to rule code LGTM.
I've requested changes to the tests (as committable suggestions) to bring them in line with our conventions.
Note: You can either refactor in this pull request (or we can create a follow-up issue) to move the duplicated code into check(property) {}
function. It's up to you.
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
@jeddy3 Sorry, I must have missed it.
That's a good idea! |
@jeddy3 The refactor has been completed. |
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 making the changes. LGTM!
I hope I am not mistaken in this refactor.
It's looks spot on to me, thank 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.
Thank you! LGTM 👍🏼
|
Closes #5945
No, it's self-explanatory.