- Notifications
You must be signed in to change notification settings - Fork928
chore: replace eslint with biome#14263
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
To avoid the large diff can we keep the tab width to 2 spaces? |
I actually feel pretty strongly that we should take this opportunity to just go all in on the defaults of our code formatter. Every setting we change is a bit self defeating, when the purpose of the tool is to eliminate code style discussions. I'd be ok with doing the tab transition in a separate PR to make this one more reviewable, but I think the plan should be to stick strongly with the defaults. As a bonus, we use tabs in our Go code because |
alwaysmeticulousbot commentedAug 14, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Meticulous spotted zero visual differences across 1412 screens tested:view results. Meticulous tested526/900 of the executable lines edited in this PR1. 1. These lines will likely automatically gain test coverage over the coming days, however if you wish to increase coverage immediately you can do so by interacting with your feature on localhost. Expected differences?Click here. Last updated for commit075c3be. This comment will update as new commits are pushed. |
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.
I smoke-tested this and didn't find any UI issues (but this was not an exhaustive test).
However, I did find a significant speedup from 54s foreslint+prettier
to 10s forbiome
.
I'm holding back approval for the FE folks, but I like the overall idea of not needing to think about formatting. I'm also a fan of the 'auto-fix' decisions that Biome makes.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
@@ -401,22 +401,18 @@ fmt/go: | |||
go run mvdan.cc/gofumpt@v0.4.0 -w -l . | |||
.PHONY: fmt/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.
I used to dofmt/prettier
when formatting markdown files.
Can you extract afmt/docs
orfmt/md
to make docs formatting easier?
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.
Actually, we still need prettier for basically everything it was doing before outside of site/, so I'll just put that job back.
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.
If the tests pass, I'm fine with these changes. I have a few questions, but I trust your judgment on when to merge them.
Questions:
- Do we need to make any changes on the Coder template side?
- Is there a VSCode plugin we should use, similar to what we did with Prettier? If so, it would be helpful to describe how this will affect our existing workflow and post a note about it in #dev.
matifali commentedAug 15, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@aslilac We can add |
Uh oh!
There was an error while loading.Please reload this page.
You can ignore whitespace changes in diffs (in GitHub too) so no harm to reviewability if we want to change the indentation now imo |
d15f16f
intomainUh oh!
There was an error while loading.Please reload this page.
This PR was just already massive. 😅 |
Goodbye ESLint, and all the config and dependencies and waiting that came with it.
Instead, we can use Biome! Has all the lint rules that I think are "important" and is sooooo much faster.