Skip to content

chore: remove eslint, use biome instead#2006

Merged
filmaj merged 14 commits intomainfrom
axe-eslint-hello-biome
Sep 5, 2024
Merged

chore: remove eslint, use biome instead#2006
filmaj merged 14 commits intomainfrom
axe-eslint-hello-biome

Conversation

@filmaj
Copy link
Copy Markdown
Contributor

@filmaj filmaj commented Sep 4, 2024 •
edited
Loading

Many lines of code, little actual change. Perhaps the only thing worth taking a look at is the biome.json file that describes the lint and formatting rules.

Changes

  • gets us out of the current tough situation where eslint v9 is new but we are stuck on v8 due to the airbnb eslint config being archived and unmaintained
  • removes eslint, adds biome instead. https://biomejs.dev/ is way faster (eslint executes in dozens of seconds while biome executes in under a second) and only a single dependency compared to the complicated eslint buildchain
  • moves all tests from .js to .ts (commonJS to ESM) <-- this took the most work, finally at least a vast majority of tests are typesafe now. caught many errors because of this (like invalid HTTP response mocks and things of the like).

@filmaj filmaj added semver:patch enhancement M-T: A feature request for new functionality tests M-T: Testing work only area:typescript issues that specifically impact using the package from typescript projects area:performance issues where performance is a meaningful concern dependencies Pull requests that update a dependency file github_actions Pull requests that update GitHub Actions code labels Sep 4, 2024
@filmaj filmaj self-assigned this Sep 4, 2024
@filmaj filmaj force-pushed the axe-eslint-hello-biome branch from 3182d5a to d17be1e Compare September 4, 2024 19:17
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 93.02857% with 183 lines in your changes missing coverage. Please review.

Project coverage is 91.61%. Comparing base (0374680) to head (becd125).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2006      +/-   ##
==========================================
+ Coverage   90.26%   91.61%   +1.34%     
==========================================
  Files          34       37       +3     
  Lines        7666     9924    +2258     
  Branches      381      633     +252     
==========================================
+ Hits         6920     9092    +2172     
- Misses        734      820      +86     
  Partials       12       12
Flag Coverage Δ
cli-hooks 95.23% <95.00%> (+0.28%) ⬆️
cli-test 95.39% <80.10%> (-1.54%) ⬇️
oauth 77.39% <75.00%> (+0.85%) ⬆️
socket-mode 58.22% <59.37%> (-1.37%) ⬇️
web-api 96.86% <95.83%> (+0.28%) ⬆️
webhook 96.65% <ø> (+1.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@filmaj filmaj marked this pull request as ready for review September 5, 2024 02:57
@filmaj filmaj requested a review from a team September 5, 2024 02:57
seratch
seratch approved these changes Sep 5, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Nice! Too many changes to review but they should be safe. Regarding the changes under packages/web-api/src/types/response/, it's fine! but we may want to add the additional step that applies biome after generating in the script.

@filmaj
Copy link
Copy Markdown
Contributor Author

filmaj commented Sep 5, 2024

Sounds @seratch , I will follow up with another PR for that shortly, and can test it out by running it in the PR, too.

@filmaj filmaj merged commit e1eeac9 into main Sep 5, 2024
@filmaj filmaj deleted the axe-eslint-hello-biome branch September 5, 2024 12:30
@filmaj filmaj mentioned this pull request Sep 17, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@seratch seratch seratch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@filmaj filmaj

Labels

area:performance issues where performance is a meaningful concern area:typescript issues that specifically impact using the package from typescript projects dependencies Pull requests that update a dependency file enhancement M-T: A feature request for new functionality github_actions Pull requests that update GitHub Actions code semver:patch tests M-T: Testing work only

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@filmaj @seratch