Skip to content
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

build: enforce sort-imports eslint rule #1147

Merged
merged 1 commit into from
Aug 5, 2022

Conversation

aryanshridhar
Copy link
Collaborator

@aryanshridhar aryanshridhar commented Aug 4, 2022

Adds the sort-import rule in codebase.

Summary -

  • Added the sort-import rule in eslint. However, it lacks the feature of auto fixing changes using eslint --fix. Hence, this is where eslint-plugin-import helps us! (It also provides better control on how imports are sorted)
  • This requires eslint-import-resolver-typescript to resolve imports in typescript files.
  • Added a new command lint:js which sort the imports in .js files.
  • Ran yarn lint which autofixes all the problems for us :)

Rules kept off -

Edit: This can be merged once large pending pull requests are merged first - to avoid large merge conflicts :)

@coveralls
Copy link

coveralls commented Aug 4, 2022

Coverage Status

Coverage remained the same at 92.508% when pulling ae12de0 on aryanshridhar:Add-EslintImportOrder into d77d3ac on electron:main.

@aryanshridhar aryanshridhar marked this pull request as ready for review August 4, 2022 18:58
src/main/main.ts Show resolved Hide resolved
src/renderer/components/editors-non-ideal-state.tsx Outdated Show resolved Hide resolved
@BlackHole1
Copy link
Member

It would have been better if some details had been optimized. 😀

Copy link
Member

@BlackHole1 BlackHole1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

@BlackHole1 BlackHole1 merged commit 31bbd38 into electron:main Aug 5, 2022
@aryanshridhar aryanshridhar deleted the Add-EslintImportOrder branch August 5, 2022 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants