-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix dev build script to update dependencies acc to semver #6157
Conversation
@danieljurek @weshaggard This PR is ready for review and made all requested changes discussed offline |
eng/tools/versioning/set-dev.js
Outdated
|
||
// Update any references to internal projects with the new versions | ||
for (const package of Object.keys(rushPackages)) { | ||
if ( |
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 filter is being executed twice. We can execute it once saving lines of code and making it so that future logic changes only have to edit one version of the filter logic.
let targetPackages = [];
for (const package of Object.keys(rushPackages)) {
if (
["client", "core"].includes(rushPackages[package].versionPolicy)
&& rushPackages[package].projectFolder.startsWith(`sdk/${service}`)
) {
targetPackages.push(package);
}
}
for (const package of targetPackages)) {
console.log("package updated = ");
console.log(package);
rushPackages = updatePackageVersion(rushPackages, package, buildId);
console.log(rushPackages[package].newVer);
}
...
@weshaggard The issue comes with js-core pipeline if i only update the dependencies of the packages we want to ship then we get mismatching dependencies in case the packages are dependent amongst themselves - I think we need to update the packages that are depndent on the packages we build as well Starting "rush update" @azure/core-asynciterator-polyfill
@azure/core-http
@azure/core-tracing
@azure/abort-controller
@azure/core-arm
@azure/core-auth
@azure/identity
@azure/logger
Found 8 mis-matching dependencies! |
8074412
to
5080a2a
Compare
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.
Clean-up the last few items and we should be good.
Combined both Daniel's script and the script from PR - #5135 commit