-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[core] Flatten grid packages folder #11946
Conversation
Localization writing tips ✍️Seems you are updating localization 🌍 files. Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️
Deploy preview: https://deploy-preview-11946--material-ui-x.netlify.app/ Updated pages: |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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.
You should be able to remove a ternary in the copyFiles.mjs
file that was due to this nested folder structure
59f14cb
to
0fb0eb4
Compare
// TODO: Remove `grid` folder to flatten the structure | ||
packageData.name.includes('grid') ? '../../../CHANGELOG.md' : '../../CHANGELOG.md', | ||
]; | ||
const filesToCopy = ['./README.md', '../../CHANGELOG.md']; |
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.
When I migrated to use the same bundling strategy as the core, this code you just removed was the only diff with the core file
So maybe you could just remove everything from this file and import the core one, like we are doing for buildTypes.mjs
But this need to be double checked of course
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'll look into this and submit a follow-up PR if necessary
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.
Unfortunately, there are other non-trivial differences between our script and the one from the core repo.
Making the copyFiles
script generic would be a significant effort and I think it's not worth it.
But I think it makes sense to at least reuse some functions from that script.
I'll submit a PR to the core repo.
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.
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
0fb0eb4
to
ee05acf
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
ee05acf
to
1555a3e
Compare
Closes #10114