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.

Sign up for GitHub

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

Jump to bottom

infra: show eslint progress #3172

Merged
merged 7 commits into from
Nov 5, 2024
Merged

infra: show eslint progress #3172

merged 7 commits into from
Nov 5, 2024

Conversation

Shinigami92
Copy link
Member

why? cause it's cool
no but seriously, with that locally you can see how long you potentially need to wait anymore

docs: https://www.npmjs.com/package/eslint-plugin-file-progress | https://github.com/sibiraj-s/eslint-plugin-file-progress

@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Oct 11, 2024
@Shinigami92 Shinigami92 self-assigned this Oct 11, 2024
@Shinigami92 Shinigami92 requested a review from a team as a code owner October 11, 2024 19:41
@Shinigami92 Shinigami92 added this to the vAnytime milestone Oct 11, 2024
@netlify Netlify
Copy link

netlify bot commented Oct 11, 2024 •
edited
Loading

✅ Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit c1506a3
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/6728e304f4b1c80008842e48
😎 Deploy Preview https://deploy-preview-3172.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov Codecov
Copy link

codecov bot commented Oct 11, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (f9466b5) to head (c1506a3).
Report is 1 commits behind head on next.

Additional details and impacted files 
@@           Coverage Diff           @@
##             next    #3172   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files        2805     2805           
  Lines      217098   217098           
  Branches      970      968    -2     
=======================================
  Hits       217018   217018           
  Misses         80       80

see 1 file with indirect coverage changes

@ST-DDT
Copy link
Member

ST-DDT commented Oct 12, 2024

It slows down a full run of eslint by 3seconds.

79 seconds -> 82 seconds, but it feels more responsive because actually see it doing something.

ST-DDT
ST-DDT previously approved these changes Oct 12, 2024
View reviewed changes
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

An optional but nice little gimmick.

eslint.config.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Oct 12, 2024
@Shinigami92 Shinigami92 force-pushed the infra-eslint-progress branch from 6b4d91a to 80c1c77 Compare October 13, 2024 11:20
@Shinigami92 Shinigami92 removed the needs rebase There is a merge conflict label Oct 13, 2024
@Shinigami92 Shinigami92 requested review from ST-DDT and a team October 13, 2024 11:20
ST-DDT
ST-DDT previously approved these changes Oct 13, 2024
View reviewed changes
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Oct 26, 2024
ST-DDT
ST-DDT previously approved these changes Nov 4, 2024
View reviewed changes
ST-DDT
ST-DDT previously approved these changes Nov 4, 2024
View reviewed changes
@ST-DDT ST-DDT removed the needs rebase There is a merge conflict label Nov 4, 2024
@ST-DDT ST-DDT requested a review from a team November 4, 2024 12:03
matthewmayer
matthewmayer previously approved these changes Nov 4, 2024
View reviewed changes
@ST-DDT ST-DDT dismissed stale reviews from matthewmayer and themself via f261329 November 4, 2024 13:41
@ST-DDT
Copy link
Member

ST-DDT commented Nov 4, 2024 •
edited
Loading

@matthewmayer Sorry, they just released an update that fixes Node 18 support and ships it's own types.

@ST-DDT
Copy link
Member

ST-DDT commented Nov 4, 2024

There seems to be an issue with the new versions.

@sibiraj-s
Copy link

sibiraj-s commented Nov 4, 2024 •
edited
Loading

There seems to be an issue with the new versions.

Can you try v2.1.1, I was manually typing the types, not generated. I made some mistakes.

@ST-DDT
Copy link
Member

ST-DDT commented Nov 4, 2024

Fixed

@ST-DDT ST-DDT requested review from matthewmayer and a team November 4, 2024 14:15
ST-DDT
ST-DDT previously approved these changes Nov 4, 2024
View reviewed changes
@ST-DDT ST-DDT requested a review from a team November 4, 2024 14:16
Shinigami92
Shinigami92 commented Nov 4, 2024
View reviewed changes
eslint.config.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT requested a review from a team November 4, 2024 15:07
@Shinigami92
Copy link
Member Author

Can't approve, because historically I opened the PR
But count me as approved ✅

@ST-DDT ST-DDT merged commit 2ec9c47 into next Nov 5, 2024
23 checks passed
@ST-DDT ST-DDT deleted the infra-eslint-progress branch November 5, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ST-DDT ST-DDT ST-DDT approved these changes

@matthewmayer matthewmayer Awaiting requested review from matthewmayer

Assignees

@Shinigami92 Shinigami92

Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Milestone
vAnytime
Development

Successfully merging this pull request may close these issues.
4 participants
@Shinigami92 @ST-DDT @sibiraj-s @matthewmayer