-
-
Notifications
You must be signed in to change notification settings - Fork 942
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
[#2829] add autofix for number-no-trailing-zeros #2947
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.
@modosc Thanks! This LGTM. It seems to address the two concerns raised with the number-leader-zero
implementation:
- explicitly checking the
node.type
. - mutating the AST outside of the value-parser loop.
} | ||
}; | ||
}; | ||
|
||
function removeTrailingZeros(input, startIndex, endIndex) { |
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.
Maybe best place utils
?
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.
Is it used by more than rule? If so, then yes I agree. Otherwise, it's probably more helpful to maintainers to keep it here.
@jeddy3 does the changelog update become part of the pull request or happen after the merge? |
After the merge (as it helps us avoid conflicts). I just realised that the guidelines aren't that clear. I'll tweak them now. |
got it - it would be cool to use something to do this automatically, maybe https://skywinder.github.io/github-changelog-generator/ ? |
We experimented with automating it, but most things ended up being more trouble than they're worth. I think the current process has little overhead. |
#2829
No, it's self explanatory