-
Notifications
You must be signed in to change notification settings - Fork 23
Switch to Oxlint/Oxfmt #353
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
base: master
Are you sure you want to change the base?
Conversation
| import ModelView from "./components/ModelView.vue"; | ||
| import ProfileUncertaintyView from "./components/ProfileUncertaintyView.vue"; | ||
| import SimpleBuilder from "./components/SimpleBuilder.vue"; | ||
| import DataView from "@/components/DataView.vue"; |
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'm a little surprised by this suggested change - it doesn't seem better to me. What is the benefit of using the special @ construction?
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 looked up this suggestion online... I can see the logic. Since we're already loading a resolver and aliases, it might be a good idea to simplify the bumps imports too from bumps-webview-client/src/... to @bumps/...
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.
not the worst idea. do you want me to test that out in this PR or do it separately?
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.
Sure, if you want to do it here that's fine!
|
I also need to see if I can't figure out why the CI types test is failing when it passes for me locally - it accurately assesses that there's an implicit type mismatch in some places, but how we'd want to address that is up for discussion |
…odel_loaded topic instead of message)
|
nice, you got it haha. |
|
I don't have much knowledge of either tool. It's nice that our dependency list gets so much smaller with this change (prettier was a bit of a bear to manage) |
|
It seems like both |
|
yeah i've tested biome again and run into the same issue as before (i didn't realize that PR was already on biome 2.3) i like that biome is a little more stringent and flexible, it's making a lot of really solid code style suggestions, and catching things that oxlint didn't (maybe we need to extend the rules?) at least oxlint seems to be respecting the template section and not throwing false positives around. |
|
oh, one thing we may want to bring over from the biome PR is that |
|
Agreed... it is just bad form to shadow globals! |
Same motivation as #342, but oxlint is by the Vue team and actually supports Vue style files, which biome currently does not support.
Full compatibility with eslint and prettier rules (no new or missing linting errors / formatting changes were made in running the new tool(s).
Will follow up with an analogous bumps PR if this is received well