tools: use stylistic ESLint plugin for formatting #50714
Conversation
|
Review requested:
|
nodejs-github-bot
added
lib / src
There was a problem hiding this comment.
Even though, the proposed change solves the current deprecated rules issue, I strongly believe that it should be interpreted as a "wake up call" to resolve the formatting issues in the repository.
None of these rules fixes the formatting issues related to the examples I've mentioned in the original pull request.
This is orthogonal. We can fix the immediate deprecation AND work on a replacement. |
The deprecated rules will not be removed until ESLint 10 (according to the blog post announcing the deprecation). So there is no immediate action needs to be taken as of today. If we might/will/can replace the Stylistic package with something else, I think we should keep the git history clean and make the decision right now. |
|
@anonrig Why there is a need to replace Stylistic package with something else? What's the urgency? |
There was a problem hiding this comment.
Can we do something similar to facebook/react#4034 and create aliases for the style rules without the prefix? Then we don't even need to update the eslint-disable comments.
FWIW I don't see those as issues. Whitespace styles can be enforced on a best-effort basis. Not indenting some function call the way some people think it should is not going to do much damage. If we can use a tool that places whitespaces in a less controversial way so that we can stop making it even a topic in code reviews, that's great. To me it doesn't really matter how complete the tool is at finding and enforcing whitespace styles, what's important is that we can just say "the tool doesn't complain" and move on and not spend time bikeshedding about whitespace styles. If the introduction of this tool creates a huge amount of diffs in the existing code base and make whitespaces any more attention-worthy than they already are in diffs, or introduce any bikeshedding about whitespace styles, then I think that's not really worth it. |
|
@joyeecheung I've done the work already. |
I might still be worth to undo that work to minimize the git diff / not pollute the git blame, and create aliases as Joyee suggested. Let us know if you'd prefer someone else do it. |
|
I certainly don't want to do it while the PR is blocked, and I would prefer if someone else does it. |
|
IMHO, it's better to keep those rules as is right now, so that people won't get confused from where those rules are came from 🙄 |
|
I don't think the |
|
I considered it blocked on the decision wrt. Biome, not on Joyee's suggestion. |
|
ESLint v9 was released. What are we going to do about it? |
|
@anonrig are you still blocking this? |
|
I'll update it asap |
targos
added
request-ci
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
removed
the
request-ci
anonrig
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in ca7c8c6 |
Refs: https://eslint.org/blog/2023/10/deprecating-formatting-rules/ PR-URL: #50714 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Refs: https://eslint.org/blog/2023/10/deprecating-formatting-rules/ PR-URL: #50714 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Refs: https://eslint.org/blog/2023/10/deprecating-formatting-rules/ PR-URL: #50714 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Refs: https://eslint.org/blog/2023/10/deprecating-formatting-rules/ PR-URL: nodejs#50714 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Refs: https://eslint.org/blog/2023/10/deprecating-formatting-rules/