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

[ENHANCMENT] Add explicit "release" identifier to inc function to strip prerelease suffix from the version. #740

Open
deej-io opened this issue Aug 20, 2024 · 5 comments

Comments

@deej-io
Copy link

deej-io commented Aug 20, 2024

I have been building a release process for one of my projects using npm-version and wanted to promote a prerelease version to a full version and it wasn't immediately obvious how to do that.

After delving into the code, I found that patch effectively does what I want, however I think this is confusing and potentially error-prone when running the command in a context where we don't know whether the current version is a pre-release or not - e.g. in a CI job.

I think it would be beneficial to have a release increment identifier that strips the prerelease suffix from the version. This is different from the patch identifier in that it does not bump the patch version in the case where there is no prerelease suffix.

Instead, this identifier could either:

  • Emit an error that the the current version is not a pre-release; or
  • Return the version unmodified

I would personally prefer it to return an error, however I am open to reasons why a no-op would be better.

I am happy to implement this and propagate the change up to npm-version.

@wraithgar
Copy link
Member

This seems very discrete and user-friendly. So the idea is that npm could then support npm version release?

One of the biggest lessons I've learned in maintaining npm is that it's always best to be explicit and never guess user intent. With that in mind I'd lean towards throwing if the version does not have a pre-release portion.

@mbtools
Copy link
Contributor

mbtools commented Aug 20, 2024

Like this?

      case 'release':
        if (!this.prerelease.length) {
          throw new Error('version is not a pre-release')          
        }
        this.prerelease = []
        break

LGTM 👍

@deej-io
Copy link
Author

deej-io commented Aug 21, 2024

This seems very discrete and user-friendly. So the idea is that npm could then support npm version release?

That's the overall intention, yeah.

One of the biggest lessons I've learned in maintaining npm is that it's always best to be explicit and never guess user intent. With that in mind I'd lean towards throwing if the version does not have a pre-release portion.

Agreed! Thank you for the feedback :)

Like this?

      case 'release':
        if (this.prerelease.length) {
          throw new Error('version is not a pre-release')          
        }
        this.prerelease = []
        break

LGTM 👍

Great! I'll get on this when I'm out from work.

@wraithgar
Copy link
Member

if (!this.prerelease.length) { I should hope, right?

@mbtools
Copy link
Contributor

mbtools commented Aug 21, 2024

yes, ! of course 😄 (fixed above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants