restechnica / semverbot

A CLI which automates semver versioning.
Mozilla Public License 2.0
137 stars 8 forks source link

blang/semver only supports "v" prefix's #58

Closed talios closed 11 months ago

talios commented 11 months ago

It looks like the upstream blang/semver module only supports v as a prefix when using ParseTolerant - so any configured prefix in semverbot doesn't appear to work.

// ParseTolerant allows for certain version specifications that do not strictly adhere to semver
// specs to be parsed by this library. It does so by normalizing versions before passing them to
// Parse(). It currently trims spaces, removes a "v" prefix, adds a 0 patch number to versions
// with only major and minor components specified, and removes leading 0s.
func ParseTolerant(s string) (Version, error) {
    s = strings.TrimSpace(s)
    s = strings.TrimPrefix(s, "v")

I wonder if it would good to add to a simple string replacement on the configured prefix to v in https://github.com/restechnica/semverbot/blob/main/pkg/semver/parse.go#L5 as a work around?

I noticed there were no tests covering prefixes either.

shiouen commented 11 months ago

Hi @talios, I was able to reproduce the issue and test your sbot version vs. the latest release. You are correct, there are no tests for prefixes and the latest release runs into trouble once a different prefix than 'v' has been pushed. I will gladly take a look at your PR.

I appreciate the contribution!

shiouen commented 11 months ago

For sake of documentation, I was able to reproduce it as follows:

  1. clone a fresh copy of semverbot
  2. change the prefix in semverbot.toml to 'test-'
  3. run sbot release version --verbose twice, first time succeeds but second time:
    18:53:37 INF getting version...
    18:53:37 INF 1.3.2
    18:53:37 INF predicting version...
    18:53:37 WRN falling back to patch mode
    18:53:37 INF releasing version...
    18:53:37 ERR error="command [git tag -a test-1.3.3 -m test-1.3.3] exited with exit status 128, output: \nfatal: tag 'test-1.3.3' already exists\n"
    FAIL: 1

    Clearly the latest test-v1.3.3 tag is not picked up.

  4. I tried the same process with your changes and it works flawlessly

For this specific example it goes wrong here, where the code tries to find the latest valid semver version, which is purely based on whether blangsemver's Parse method fails. Sadly, I do see the blangsemver error is silenced, this is perhaps something that should be changed in the future. Some verbose information about this would have been better.

talios commented 10 months ago

Cheers!