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

feat: adds "semver" versioning-strategy #2451

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

reggi
Copy link

@reggi reggi commented Dec 11, 2024

First off 🫶 thank you for woking on release-please it makes our lives much easier.

I work on the npm cli which uses release-please. I think its safe to say we're power users and rely on the prerelease functionality.

This PR adds a proper "semver" strategy. There's a lot of issues related to prereleases when it comes to the existing PrereleaseVersioningStrategy and DefaultVersioningStrategy (more below), these can be fixed but I think still will not do "proper" / "intended" semver incrementation.

There are also issues with semver itself which we aim to fix in https://www.npmjs.com/package/semver in a future release, this pr shims that functionality for now with a custom semver.inc impementation.

I'll copy some of my findings from npm/template-oss#495 but keep in mind the context:

  • I was unable to get a repo into prerelease using the latest version of release-please on my personal project reggi/packages.
  • And the npm/cli has been stuck in prerelease mode. It was able to get itself into prerelease because of the wrapper around release-please we have in this repo. (trail: npm/cli workflow, Strategy Turnery)

I do not believe release please has this options.prerelease ternary anymore here's the mapping of what versioning-strategy is uses.

And even if it did choose to use PrereleaseVersioningStrategy or DefaultVersioningStrategy neither of these strategies have the ability to leave a prerelease. Because they always add the current prereleaseType or prereleaseType from the config from the existing version. Neither of these classes even have access to the options.prerelease boolean. And at the point in which both have it whats the point in differentiating between them anymore?

This is added as a new versioning-strategy semver. To note the property currently documented as versioning-strategy in the docs is not working, and is currently versioning. I opened a PR to fix the json schema for the config, but now we know it's a bug i'll leave that discussion open there.

So "versioning-strategy": "semver" would be ignored and you'd need "versioning": "semver", again not relevant to this PR but another quirk to note.

I'm going to keep this in draft for now, just want to open up for discussion, and next steps. My main questions are

  • Is the release-please team open to new "semver" versioning strategy?
  • How could we fix the existing strategies? I think they're design overall is broken (unless I'm missing something) but perhaps outside the context of semver / node people rely on that functionality? Is there a world in which we can deprecate PrereleaseVersioningStrategy or DefaultVersioningStrategy and just use SemverVersioningStrategy?

I also have a published fork if anyone wants to try it https://www.npmjs.com/package/@reggi/release-please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant