-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Popper] Move from mui-material to mui-core #28923
Conversation
@material-ui/unstyled: parsed: +43.84% , gzip: +43.47% |
…per and remove inaccurate bundle size from Unstyled section.
@michaldudak aside from the Azure Devops check that failed (I'm actively trying to figure out why), the PR is ready for first round of feedback/review. Thank you! 😄 |
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 could notice few things still missing. Great first iteration 👍
Ready for another round of feedback! 😃 Any suggestions on how I might go about troubleshooting the Azure bundle size error? I read within the contribution guidelines that its probably due to the packages or how the documentation was built. 🤔 Perhaps I'm missing a step on building a bundle? |
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.
Looks good to me. The Azure Pipelines are failing with JavaScript heap out of memory
, I would ignore it for now.
Why? This job is an integral part of the CI pipeline. You can't just ignore parts because they fail. CI isn't just there to affirm you but also to signal mistakes you make. I've heard this a couple of times now from you and it's a concerning sentiment. I don't understand why you say this repeatedly. If you think there's a problem with CI in general, then let's discuss this separately. But constantly saying "ignore CI" in PRs is not a heatly sentiment. There is definitely something off here that needs to be adressed. |
I said, " I would ignore it for now". I meant for this iteration of the review, there are still things that need to be polished, we can focus on it in the end. I am not going to merge the PR if the CI is not green. |
Talking about the Azure Pipelines, we should move the condition for the |
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.
Looks good. Nice work 👍🏻
Congratulations @rebeccahongsf on being the first community contributor to create an unstyled component in @mui/core! 🎉 |
Moved the Popper component to the unstyled package and updated all references.