Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: replace globby with tinyglobby#9878

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

benmccann
Copy link
Contributor

PR Checklist

Overview

Replacesglobby withtinyglobby for a smaller more efficient implementation with fewer dependencies

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@benmccann!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

@netlifyNetlify
Copy link

netlifybot commentedAug 23, 2024
edited
Loading

Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commitb0e7392
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/66cb416250358f00088bfb07
😎 Deploy Previewhttps://deploy-preview-9878--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 95 (🔴 down 4 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 90 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

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

@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedAug 23, 2024
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commitb0e7392. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 30 targets

Sent with 💌 fromNxCloud.

@benmccannbenmccannforce-pushed thetinyglobby branch 2 times, most recently from5a768a5 toc53b295CompareAugust 24, 2024 04:07
@Josh-Cena
Copy link
Member

Why do we have both this and#9518?

@benmccann
Copy link
ContributorAuthor

Oh, thanks for pointing that out. I hadn't seen it. I thinktinyglobby is better as it has many fewer dependencies, but that PR is passing the CI whereas this one isn't. I'll check if the author is able to switch totinyglobby in the other PR and if so perhaps we can close this one

@benmccannbenmccannforce-pushed thetinyglobby branch 2 times, most recently fromda172bf to4fc5572CompareAugust 24, 2024 14:24
@SukkaW
Copy link
Contributor

It seems that your CI failed due to incorrect spying of glob functions. Might wanna check that out.

@rubiesonthesky
Copy link
Contributor

I just want to point out that this change added 2 new dependencies to the project and did not actually remove any included dep, if you check yarn.lock. So this feels kinda pointless change this time.
(note, I’m just a user of this plugin, so my opinions do not reflect what the maintainers may think)

@benmccann
Copy link
ContributorAuthor

benmccann commentedAug 24, 2024
edited
Loading

I just want to point out that this change added 2 new dependencies to the project and did not actually remove any included dep, if you check yarn.lock. So this feels kinda pointless change this time.
(note, I’m just a user of this plugin, so my opinions do not reflect what the maintainers may think)

Theyarn.lock isn't what's important here because it includes bothdependencies anddevDependencies.globby remains only for developers of this project. If you look at where it's used now it's in things like docusaurus. This PR would remove about 40% of dependencies for all consumers oftypescript-eslint going from 44 down to 26

@benmccann
Copy link
ContributorAuthor

It seems that your CI failed due to incorrect spying of glob functions. Might wanna check that out.

I found the line I missed updating before. It's now correctly mocking. However, it's returning things in a different order. I think we basically have to combine our two PRs to get it working

@benmccannbenmccannforce-pushed thetinyglobby branch 2 times, most recently from6c5e2d7 tobf84804CompareAugust 24, 2024 20:34
@SukkaW
Copy link
Contributor

However, it's returning things in a different order.

See#9518 (comment)

@bradzacherbradzacher linked an issueAug 25, 2024 that may beclosed by this pull request
4 tasks
@codecovCodecov
Copy link

codecovbot commentedAug 25, 2024
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.03%. Comparing base(9f70ed1) to head(b0e7392).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@##             main    #9878   +/-   ##=======================================  Coverage   88.03%   88.03%           =======================================  Files         407      407             Lines       13941    13944    +3       Branches     4071     4071           =======================================+ Hits        12273    12276    +3  Misses       1355     1355             Partials      313      313
FlagCoverage Δ
unittest88.03% <100.00%> (+<0.01%)⬆️

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

FilesCoverage Δ
...ipt-estree/src/parseSettings/resolveProjectList.ts97.87% <100.00%> (+0.14%)⬆️

@bradzacher
Copy link
Member

As per#9453 (comment) we're going to go with#9518 for now.
We may revisit this in future and switch fromfast-glob totinyglobby.

@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsSep 3, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@bradzacherbradzacherbradzacher requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhancement: replaceglobby withfast-glob
5 participants
@benmccann@Josh-Cena@SukkaW@rubiesonthesky@bradzacher

[8]ページ先頭

©2009-2025 Movatter.jp