| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Add sort-keys rule in EsLint. Related issue: nodejs#41767 CC:@nodejs/linting
|
CC: @nodejs/linting |
Sorry, something went wrong.
|
I agree with the sentiment, the problem being the diff would be way too large for it to be worth it. Ideally we would have another set of "aspirational" lint rules that would apply only on code being added, but I'm not sure it exists a tool that would allow us to do that. |
Sorry, something went wrong.
|
It might be better to set it as warn-ing ⚠️ |
Sorry, something went wrong.
|
Warnings wouldn't help much in our case, it would make ESLint output very noisy, and the auto-fixes would probably still produce that large diff I'd like to avoid. |
Sorry, something went wrong.
|
Instead, can we go for error and run --fix for them? |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for this (and all your recent work it's appreciated!)
Blocking so this doesn't land without consensus which we didn't have last time.
Sorry, something went wrong.
Sure. Let's invite opinions. Thanks @benjamingr |
Sorry, something went wrong.
I think it's the same consensus as regarding trailing commas: we'd like to have them used everywhere, but it's not worth accepting PRs focusing on adding them. While improving the consistency of the code base is great, landing commits that do stylistic changes pollute the git blame output, without improving the quality of the codebase otherwise. If you are able to limit the scope to a smaller chunk of the code base that already complies with this rule, sure that would certainly be accepted, but you have to keep in mind that humans review the PRs, so a PR that introduces thousands of changes across the whole codebase is probably too much to ask. |
Sorry, something went wrong.
Even if we had that, I would be -1 on enabling this rule with it. |
Sorry, something went wrong.
|
Closing the MR for the time being as discussed the unintended issues behind this change. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Add sort-keys rule in EsLint.
Related issue: #41767
CC: @nodejs/linting