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

Remove left over ESLint artifacts #7743

Merged
merged 8 commits into from
Apr 11, 2024
Merged

Remove left over ESLint artifacts #7743

merged 8 commits into from
Apr 11, 2024

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Apr 10, 2024

With the recent switch to Biome for linting (#7665) we removed eslint. Some plugins and config files were left behin. This PR removes them.

Replaced circular dependency detection tool from madge to dpdm since I ran into dependency problems. Newer versions of madge would detect 100+ circular imports, mostly due to type imports which were not reliably ignored. (see this Madge issue

Steps to test:

  • None. CI is enough

Issues:


(Please delete unneeded items, merge only when none are left open)

@hotzenklotz
Copy link
Member Author

hotzenklotz commented Apr 10, 2024

The CI fails on on checking the circular dependencies :-/ Circle checks are done with a tool called madge

Some observations:

  • We currently use madge v5.x which discovers 8 circles
  • Madge v6.x produces 118 circles on this branch and different feature branch (better-mesh-context-menu)
    • The existence of the eslint config, which are being removed in the PR, does not seem to have any influence since the circles are detected on this branch and the features brunch which still has those files.
    • Many circles are more or less the same circle repeated.
  • Madge v7.0.0 (latest) does not detect any circles.
    • Warning that the TS version 5.4 is not supported. (but TS 5.4 is installed because of a direct dependency of madge 🤦 )

@hotzenklotz hotzenklotz requested review from philippotto and removed request for MichaelBuessemeyer April 10, 2024 14:03
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Great 👍 While you are at it, you could also remove a bunch of // eslint-disable comments that still linger around..

@hotzenklotz hotzenklotz enabled auto-merge (squash) April 11, 2024 12:04
@hotzenklotz
Copy link
Member Author

Great 👍 While you are at it, you could also remove a bunch of // eslint-disable comments that still linger around..

Ok, done. See commit: 8b79f07

@hotzenklotz hotzenklotz enabled auto-merge (squash) April 11, 2024 12:17
@hotzenklotz hotzenklotz merged commit 2872574 into master Apr 11, 2024
2 checks passed
@hotzenklotz hotzenklotz deleted the remove-eslint branch April 11, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants