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

Feature/add import order rule to eslint #2778

Conversation

miettal
Copy link
Contributor

@miettal miettal commented Sep 3, 2024

In a96c720, vscode import organize setting is configured.
This PR add same behavior setting to eslint.

  1. import/order is import order rule
  2. in default, import/order categorize import like builtin, external internal... and these are separate by each group.
  3. But vscode import organization setting don't care it. and the function make 1 group.
  4. in this setting, this rule customized by definition all import in 1 group to fit vscode import organization setting.

@miettal miettal marked this pull request as draft September 3, 2024 00:46
Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files
@@                  Coverage Diff                  @@
##             develop-2024-10-07    #2778   +/-   ##
=====================================================
  Coverage                 56.01%   56.01%           
  Complexity                 8197     8197           
=====================================================
  Files                      2103     2103           
  Lines                     89017    89017           
  Branches                   6560     6560           
=====================================================
  Hits                      49853    49853           
  Misses                    37446    37446           
  Partials                   1718     1718           

@miettal
Copy link
Contributor Author

miettal commented Sep 3, 2024

Error: src/app/edge/history/shared.ts:3:62 - error TS2305: Module '"date-fns"' has no exported member 'de'.

wow, I just added eslint rules and only did eslint --fix src/. but error is happened. so I convert this PR to draft. I will check it.

@miettal
Copy link
Contributor Author

miettal commented Sep 4, 2024

Error: src/app/edge/history/shared.ts:3:62 - error TS2305: Module '"date-fns"' has no exported member 'de'.

wow, I just added eslint rules and only did eslint --fix src/. but error is happened. so I convert this PR to draft. I will check it.

This is eslint-plugin-import bug..
import-js/eslint-plugin-import#1479

@miettal miettal marked this pull request as ready for review September 4, 2024 13:52
@lukasrgr
Copy link
Contributor

@miettal are these settings recommended?

@miettal miettal force-pushed the feature/add-import-order-rule-to-eslint branch from d3a7ef4 to 22743ab Compare October 2, 2024 15:22
@miettal
Copy link
Contributor Author

miettal commented Oct 2, 2024

I rebased this PR to catch up 2024.10 release.

I'm not familiar with bestpractice for typescript import order rules.
But in fork project, current setting will generate many import order diffs from upstream by using vscode editing. this is bad effect to openems ecosystem. its make difficult to management patchset. this is sure.
So I think unifying import order in all codes and forcing import rule are better.

@sfeilmeier sfeilmeier changed the base branch from develop to develop-2024-10-07 October 13, 2024 12:05
@sfeilmeier sfeilmeier merged commit cf109b4 into OpenEMS:develop-2024-10-07 Oct 13, 2024
7 checks passed
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.

3 participants