-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Codemod: Replace globby with tinyglobby
#31407
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
|
View your CI Pipeline Execution ↗ for commit 6a5711c
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 219 | 204 | 🎉 -15 🎉 |
| Self size | 879 KB | 879 KB | 🚨 +90 B 🚨 |
| Dependency size | 81.81 MB | 81.58 MB | 🎉 -228 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 173 | 🎉 -15 🎉 |
| Self size | 35 KB | 35 KB | 🚨 +54 B 🚨 |
| Dependency size | 76.88 MB | 76.66 MB | 🎉 -228 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
|
Hey @benmccann thanks for the PR In the issue it was mentioned about something important:
Can you check whether that is already supported by tinyglobby? |
|
Ah, thanks. I only read the issue description and missed the comment. Unfortunately tinyglobby doesn't yet have support for that. I'm not sure if there's a large codebase you'd like to test this on to see if it matters or not. Although, I wonder if it wouldn't be an issue already since the addons package uses it? |
aba54ed to
fc110a5
Compare
|
@yannbf I just realized there very clearly won't be any performance regression in this PR related to that feature because |
fc110a5 to
3f0af66
Compare
Hey Ben! I'm sorry for taking so long to reply. We do use the Regardless, I spoke with the team about this and we are planning on (@JReinhold suggestion):
|
|
Thanks! Yes, I did see the This PR was meant as part of the gradually migrate on a per-package level approach, which is what I'd had in mind as well. I'm not sure if this particular package needs to be migrated in a major or not, but hopefully this PR should be good to go whenever you are ready to accept it. |
globby with tinyglobby
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I manually QA'ed this in a small project with
A. A regular file
B. A file that was gitignored
C. A file that was symlinked into the directory the glob matched
All files where modified similar to the existing behavior.
Great job, and thanks for the patience! 🙏
Progress towards #29227
What I did
The
addonsproject has been migrated totinyglobbyalready. This updates thecodemodpackage as wellChecklist for Contributors
Testing
I'm assuming the
codemodlibrary is already covered by tests. I'm not sure what kindThe changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
This PR replaces the
globbypackage withtinyglobbyin the Storybook codemod package, part of an effort to streamline dependencies across the Storybook ecosystem.globbyv14.0.1 withtinyglobbyv0.2.13 incode/lib/codemod/package.jsonglobbytotinyglobbyincode/lib/codemod/src/index.tsglobandglobbywithtinyglobby#29227 for optimizing glob-related dependencies💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!