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

Fix #6487 specify which Sandpack files go into a src directory #6496

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Fix #6487 specify which Sandpack files go into a src directory #6496

merged 2 commits into from
Dec 15, 2023

Conversation

geeseyj
Copy link
Contributor

@geeseyj geeseyj commented Dec 15, 2023 •
edited by mattcarrollcode
Loading

Preface: #6487

This PR removes the assumption that all Sandpack example files should be put into a /src/ folder and instead does so explicitly on only that files that need it.

preview

@geeseyj geeseyj mentioned this pull request Dec 15, 2023
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 15, 2023 •
edited
Loading

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@geeseyj geeseyj changed the title Fix #6487 specify which sandbox files go in src directory Fix #6487 specify which Sandpack files go into a src directory Dec 15, 2023
@mattcarrollcode mattcarrollcode self-requested a review December 15, 2023 20:44
@mattcarrollcode
Copy link
Contributor

@geeseyj Thanks for identifying and writing up a fix! This looks like a lot of monotonous work, thank you for doing this. The general approach looks good I'm taking a closer look now.

@geeseyj
Copy link
Contributor Author

geeseyj commented Dec 15, 2023

I just found a few stragglers 😅 🤦 un momento

mattcarrollcode
mattcarrollcode approved these changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@mattcarrollcode mattcarrollcode left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, merging

@mattcarrollcode mattcarrollcode merged commit bb65ace into reactjs:main Dec 15, 2023
4 checks passed
@geeseyj
Copy link
Contributor Author

geeseyj commented Dec 15, 2023

Thanks!

@geeseyj Thanks for identifying and writing up a fix! This looks like a lot of monotonous work, thank you for doing this. The general approach looks good I'm taking a closer look now.

Luckily regexr was able to help me out hah regexr.com/7p78p

@harish-sethuraman harish-sethuraman mentioned this pull request Dec 16, 2023
Merged
@drumst0ck drumst0ck mentioned this pull request Dec 18, 2023
Closed
@rickhanlonii rickhanlonii mentioned this pull request Feb 6, 2024
Closed
@mattcarrollcode mattcarrollcode mentioned this pull request Feb 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattcarrollcode mattcarrollcode mattcarrollcode approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@geeseyj @mattcarrollcode @facebook-github-bot