-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Bump internal packages #13479
Bump internal packages #13479
Conversation
Deploy preview: https://deploy-preview-13479--material-ui-x.netlify.app/ |
@@ -62,7 +62,7 @@ | |||
"react-dom": "^17.0.0 || ^18.0.0" | |||
}, | |||
"devDependencies": { | |||
"@mui/internal-test-utils": "1.0.0", | |||
"@mui/internal-test-utils": "^1.0.1", |
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.
Do you know why we define this one in each package and not just at the root?
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.
Maybe because mui/material-ui
has it set up like that... 🤷 🙈
Not sure, I also don't see too much point in that.
Maybe @michaldudak could clarify as he introduced it with #12880.
I've pushed a commit keeping only root dependency to see if everything sill works after that: 7a74141.
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.
Because it's used in each of these packages. It may not be a big deal for devDependencies, but it's a good practice to define dependencies where they are used. This way packages can be more self-sustained and not implicitly depend on anything around them.
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 appreciate the clarification.
Personally, I don't have a strong preference on this, but I understand the position, especially if at some point the package would have to be moved from the monorepo context, there would be one less thing to adjust.
WDYT @flaviendelangle?
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.
That's fine for me
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
@mui/internal-*
packages.mui/code-infra how would you suggest handling these package bumping/syncing while the core repo is in thenext
phase? 🤔All the new packages are released with the
next
tag, thus, they will not be bumped byrenovate
. 🤷Disregard the ☝️ comment as it seems that renovate doesn't look at the tag, but bumps to the highest available
semver
number.The only reason it didn't bump all packages is the concurrent PRs limit of 30, which we could consider increasing. 🤔
mui-x/renovate.json
Line 108 in 29b9131