-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[EngSys] eng/tools/versioning/set-dev.js fixes #36084
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR fixes two issues in the engineering system's versioning script and improves logging with indentation:
- Adds "utility" to the version policy check so utility packages get dev versions
- Changes version comparison logic from exact equality to range satisfaction using
semver.satisfies()
- Improves logging by adding indentation through
logPrefix
parameters for better readability
This PR addresses two issues: - we should set dev versions for utility packages too but currently don't because of a missing category in the version policy check. - we should use dev version of a package for its dependents if its current version satisfies their dependency version range requirements. Currently we use its dev version if its current version equals the minimum version of dependency version range requirements. While at this, logging is also improved by introducing indentations for nested log lines.
79a3989
to
e38345a
Compare
if (semver.eq(parsedDepMinVersion, parsedPackageVersion)) { | ||
repoPackages = updatePackageVersion(repoPackages, depName, buildId, catalogs); | ||
console.log(`${logPrefix}resolved version = ${resolvedVersion.resolution.specifier}`); | ||
if (semver.satisfies(packageVersion, resolvedVersion.resolution.specifier)) { |
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.
Changing to use semver.satisfies
const parsedDepMinVersion = semver.minVersion(depVersionRange); | ||
if (semver.eq(parsedDepMinVersion, parsedPackageVersion)) { | ||
repoPackages = updatePackageVersion(repoPackages, depName, buildId, catalogs); | ||
if (semver.satisfies(packageVersion, depVersionRange)) { |
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.
Changing to use semver.satisfies
if (resolvedVersion.type === "found") { | ||
const parsedDepMinVersion = semver.minVersion(resolvedVersion.resolution.specifier); | ||
shouldReplace = semver.eq(parsedDepMinVersion, parsedPackageVersion); | ||
shouldReplace = semver.satisfies(packageVersion, resolvedVersion.resolution.specifier); |
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.
Changing to use semver.satisfies
} else { | ||
const parsedDepMinVersion = semver.minVersion(depVersionRange); | ||
shouldReplace = semver.eq(parsedDepMinVersion, parsedPackageVersion); | ||
shouldReplace = semver.satisfies(packageVersion, depVersionRange); |
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.
Changing to use semver.satisfies
for (const pkg of Object.keys(repoPackages)) { | ||
if ( | ||
["client", "core", "management"].includes(repoPackages[pkg].versionPolicy) && | ||
["client", "core", "management", "utility"].includes(repoPackages[pkg].versionPolicy) && |
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.
adding the missing utility
This PR addresses two issues:
we should set dev versions for utility packages too but currently don't because of a missing category in the version policy check.
we should use dev version of a package for its dependents if its current version satisfies their dependency version range requirements. Currently we use its dev version if its current version equals the minimum version of dependency version range requirements.
While at this, logging is also improved by introduce indentations for nested log lines.