Skip to content
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

resurrect @lerna/conventional-commits patch to avoid bumping major version unnecessarily #8242

Closed
warner opened this issue Aug 23, 2023 · 0 comments · Fixed by #8243
Closed
Assignees
Labels
enhancement New feature or request tooling repo-wide infrastructure

Comments

@warner
Copy link
Member

warner commented Aug 23, 2023

What is the Problem Being Solved?

Once upon a time, we had a patch-package -type patch to @lerna/conventional-commits:

https://github.com/Agoric/agoric-sdk/blob/415aebe2b1b3753125328ce059d8fd032794bafb/patches/%40lerna%2Bconventional-commits%2B3.18.5.dev.patch

to add the following clause:

         // result might be undefined because some presets are not consistent with angular
         // we still need to bump _something_ because lerna saw a change here
-        const releaseType = data.releaseType || "patch";
+        let releaseType = data.releaseType || "patch";
+        
+        // Don't gratuitously break compatibility with clients using `^0.x.y`.
+        if (semver.major(pkg.version) === 0) {
+          if (releaseType === "major") {
+            releaseType = "minor";
+          } else if (releaseType === "minor") {
+            releaseType = "patch";
+          }
+        }

to reduce the version bump that our MAINTAINERS.md recipe caused when we ran yarn lerna version --no-push --conventional-graduate. This clause appears just before an if statement which does one thing with a prereleaseId, and a different thing without.

In #5034, we upgraded our use of lerna to version 3.22.1, which includes basically the same patch, except only in the non-prereleaseId arm of the if statement. It was this version of the tree that was branched to create the release-mainnet1B branch, and was used for the vaults/bulldozer release.

Then later, in 1c217cf , we upgraded to lerna version 5.6.2.

Our agoric-upgrade-11 release will be made using yarn lerna version --no-push --conventional-prerelease --preid u11, from a branch derived from release-mainnet1B. This branch lacks our original patch, and the prereleaseId arm does not perform the releaseType reduction, which means our release process would bump some packages to higher (albeit still prerelease) versions than we would want. In particular, @agoric/cosmos would be bumped from 0.34.1 to 1.0.0-u11.0. We'd prefer to see a smaller version bump, so that the sequence of published versions remains fairly monotonic.

(Our next anticipated release, agoric-upgrade-12, will be from trunk, without the "prerelease" flag, so we expect it to claim 0.35.0 or something)

By applying our original patch to the current version of lerna, we can reduce the releaseType level for both arms. In testing, this will get us an @agoric/cosmos version of 0.35.0-u11.0, which sorts in between the current 0.34.1 and the anticipated -upgrade12 value of 0.35.0.

Description of the Design

My plan is to reinstate the old patch to node_modules/@lerna/conventional-commits/lib/recommend-version.js, and commit it with patch-package. Then I'll chery-pick this commit to the -upgrade11 branch (with changes, since the base version of lerna is different on that branch).

I'll cherry-pick in that direction, rather than performing the change on the branch and then merging the branch back, because we anticipate problems with that merge, and have decided to not do it for this release/branch. The requirement we induce by making that choice is that we should not do any original development on the branch, for fear of omitting something on trunk (and thus regressing in a future release).

cc @kriskowal @turadg @mhofman , as folks involved in the original patch, upgrading of lerna, and/or the agoric-upgrade-11 effort.

Security Considerations

none, only affects generated/published/tagged package versions

Scaling Considerations

none

Test Plan

it is sufficient to eyeball the versions proposed by lerna version before approving the action, during our release process

Upgrade Considerations

none

@warner warner added enhancement New feature or request tooling repo-wide infrastructure labels Aug 23, 2023
@warner warner self-assigned this Aug 23, 2023
@warner warner changed the title resurrect lerna patch to avoid bumping major version unnecessarily resurrect @lerna/conventional-commits patch to avoid bumping major version unnecessarily Aug 23, 2023
warner added a commit that referenced this issue Aug 23, 2023
…ereleases

We used to patch this library to reduce the `releaseType` level for
our packages that had not yet reached 1.0 . We then switched to an
upstream version which performed this reduction itself, except not
when doing a "prerelease". Now that we need to do a prerelease, we'd
like this reduction behavior in both arms of the conditional, so this
commit re-introduces our original patch.

closes #8242
warner added a commit that referenced this issue Aug 24, 2023
…ereleases

(cherry-picked from commit b1917d6
from trunk, contained in PR #8243)

We used to patch this library to reduce the `releaseType` level for
our packages that had not yet reached 1.0 . We then switched to an
upstream version which performed this reduction itself, except not
when doing a "prerelease". Now that we need to do a prerelease, we'd
like this reduction behavior in both arms of the conditional, so this
commit re-introduces our original patch.

refs #8242
anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this issue Feb 16, 2024
…ereleases

We used to patch this library to reduce the `releaseType` level for
our packages that had not yet reached 1.0 . We then switched to an
upstream version which performed this reduction itself, except not
when doing a "prerelease". Now that we need to do a prerelease, we'd
like this reduction behavior in both arms of the conditional, so this
commit re-introduces our original patch.

closes Agoric#8242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tooling repo-wide infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant