-
Notifications
You must be signed in to change notification settings - Fork 13
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
Sort all imports #101
Sort all imports #101
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.
Looks good to me, thank you Colin!
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.
Thanks @colin-grant-work!
I also don't have any hard feelings about the concrete order of imports. To me the most important aspect is that we don't need to care about the order, unnecessary imports and don't introduce noise when we run "Organize Imports" locally. Because I personally rarely edit the imports manually, I just run quick fixes to add imports and then "Organize Import".
So I actually would propose to also enable organizing the imports on save in the settings.json
and make sure this works nicely with the respective eslint config:
{
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": "explicit",
"source.organizeImports": "explicit"
},
"eslint.validate": [
"javascript",
"typescript"
],
}
To make this work locally, I needed to slightly adjust the settings in the errors.eslintrc.json
(see inline suggestion) and then run another npm run lint --fix
. After that everything seemed to work nicely for me locally.
We may want to consider to add the vscode-eslint
extension as a recommendation for the workspace.
{
"recommendations": [
"dbaeumer.vscode-eslint"
]
}
What do you think?
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.
Thank you @colin-grant-work! It now works fine when retrying with a cleanly started VS Code (see comment above).
a11efba
to
d3d7351
Compare
What it does
Fixes #98
I don't have strong feelings about import ordering, but I do have strong feelings about extraneous lines in a diff, and some people have local setups that automatically sort imports in different ways. This PR introduces lint rules that should enforce deterministic ordering of imports and applies them across the code base so that in the future, we should have a common standard for import ordering and we shouldn't run into cases where a PR spontaneously introduces a new order.
Others may have strong feelings: they may want a different order, or not want these lint rules at all. Please speak up if so.
How to test
Review checklist
Reminder for reviewers