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

Migrate storybook from globby to tinyglobby #144

Open
benmccann opened this issue Dec 19, 2024 · 5 comments
Open

Migrate storybook from globby to tinyglobby #144

benmccann opened this issue Dec 19, 2024 · 5 comments
Labels
accepts prs The maintainer of the target repo was contacted and welcomes prs has issue An issue for contact or tracking was created in the target repository has pr This issue has a corresponding PR in the target repo pr needs update The PR in the target repo has gone stale and needs to be rebased

Comments

@benmccann
Copy link

Splitting this one out from the umbrella issue so that we can label it as approved by the repo owners.

There is an issue for it at storybookjs/storybook#29227 and an attempt at doing it in storybookjs/storybook#29271 which had to be abandoned due to time constraints.
It's probably stale at this point, but could be a good starting point for anyone that wants to pick it up.

Originally posted by @JReinhold in #139 (comment)

@JReinhold one thing I wonder is if sub-projects could be migrated one-at-a-time or if you prefer a PR that tackles it all at once?

@Fuzzyma Fuzzyma added has issue An issue for contact or tracking was created in the target repository has pr This issue has a corresponding PR in the target repo accepts prs The maintainer of the target repo was contacted and welcomes prs pr needs update The PR in the target repo has gone stale and needs to be rebased labels Dec 19, 2024
@43081j
Copy link
Collaborator

43081j commented Dec 19, 2024

FYI i caught up the branch from next. so if anyone wants to pick it up, at least it is no longer massively outdated

@Fuzzyma
Copy link
Collaborator

Fuzzyma commented Dec 19, 2024

No idea how to label that because the pr is not up to date lol :D

@43081j
Copy link
Collaborator

43081j commented Dec 20, 2024

no label needed i think, just making sure the PR is up to date in case anyone wants to try continue it 👍

it looks like the branch has most of the work done and just needs some test runs to find any gaps

@JReinhold
Copy link

one thing I wonder is if sub-projects could be migrated one-at-a-time or if you prefer a PR that tackles it all at once?

Doing it incrementally across multiple PRs sounds fine to me. The only thing we have to consider, is that Storybook is a complicated web of packages working together, and there is a risk that multiple packages would co-exist in a flow where they both used glob/globby in the same flow. In such a case, migrating only one of them to tinyglobby could result in a mismatch of files matched, which could break the flow.
I'm not saying this is ideal, nor that this problem exists in reality, but tech debt exists and so I wouldn't be surprised if it happened. Just something to be mindful of if trying an incremental migration.

@benmccann
Copy link
Author

If there was a mismatch in files matched it seems like it'd be due to a bug in one of the globbing packages as they should be returning the same results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepts prs The maintainer of the target repo was contacted and welcomes prs has issue An issue for contact or tracking was created in the target repository has pr This issue has a corresponding PR in the target repo pr needs update The PR in the target repo has gone stale and needs to be rebased
Projects
None yet
Development

No branches or pull requests

4 participants