Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Deploying agoric-sdk with
|
| Latest commit: |
62bcab8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3a0041c5.agoric-sdk.pages.dev |
| Branch Preview URL: | https://ta-lerna-7.agoric-sdk.pages.dev |
Hmm, that's a good question. I guess I'd want a head-to-head comparison of the non- Also adding @mhofman for further feedback. |
This exactly. I also remember a previous attempt at upgrading lerna that had to be reverted / abandoned. Might be worth digging that out to see what went wrong then. |
mhofman
left a comment
There was a problem hiding this comment.
Looking into it, the previous issue when upgrading to 5 was with pre-release semver, which was addressed by a patch we're removing here. I'd like to see some validation of that the behavior we want is preserved.
| + // 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"; | ||
| + } | ||
| + } | ||
| + |
There was a problem hiding this comment.
Is this change no longer necessary?
There was a problem hiding this comment.
correct, it's one of the upstream fixes I mentioned https://github.com/lerna/lerna/blob/28c8ef232d29085ce9a9ba2bff96fae9017b3963/libs/core/src/lib/conventional-commits/recommend-version.ts#L76-L81
There was a problem hiding this comment.
That check is only applicable for non pre-release, which is the problem we had with lerna 5, See #8242
There was a problem hiding this comment.
I elected to ramp all packages into non-zero major versions rather than maintain the Lerna patch going forward.
There was a problem hiding this comment.
Isn't the problem that lerna will elect to use a new 1.x version for pre-release but when cutting the final release it will keep using a 0.x version?
| "eslint-plugin-prettier": "^5.1.3", | ||
| "eslint-plugin-require-extensions": "^0.1.3", | ||
| "lerna": "^5.6.2", | ||
| "lerna": "^7.4.2", |
There was a problem hiding this comment.
Out of curiosity, why stop at 7 and not go all the way to 8 ?
There was a problem hiding this comment.
Not wanting to take additional risk. v7 has enough to drop the patches.
https://github.com/lerna/lerna/blob/main/CHANGELOG.md#breaking-changes seems like it wouldn't require more work but it also isn't compelling
|
Superseded by #11503 |
closes: #4660
Description
Instead of removing Lerna, double down. Upgrade to v7.
https://github.com/lerna/lerna/blob/main/CHANGELOG.md#700-2023-06-08 lists breaking changes:
The two local patches are no longer necessary because they have been fixed.
Security Considerations
Adopting NX could be problematic but this disables it.
Scaling Considerations
n/a
Documentation Considerations
Should work exactly the same.
Testing Considerations
I don't know how to safely validate the MAINTAINERS.md instructions. Looking to @gibson042 for what he'd want to see for confidence it will work.
The
lerna publishcommands are tested by some CI integration tests.