Repository: goreleaser/goreleaser
Pull request #871 (2018-11-11 21:33:13)

Add check if tag contains indicator for pre release
This PR addresses #667 
It's not complete yet but with a draft for the implementation.

The changes enable `goreleaser` to automatically mark a release as a pre release when the tag contains an indicator for it. A tag that would lead to a pre release would be `0.0.1-rc1`. The requirement is basically that `-` followed by 1+ characters is present.
This can probably be tweaked and hardened a bit. I'm open for suggestions!

I'm also not 100% sure yet if this is the best location for the code.
`ctx.Config.Release.Prerelease` is not accessed anywhere else so the choice where to put the code is basically free. But since we change the datatype from `bool` to `string` and `Context` does not have a member for `Prerelease` we would have to carry the variable around otherwise.

So either we check quite late in the stack (as I did) or we could add a Member to `Context` and have the check after reading the config file. I think the latter would be a better way but to demo my idea I implemented the first choice.

Todo:
- [x] Add tests
- [x] Clarify when to check for pre release in tag


codecov-io: # [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=h1) Report
> Merging [#871](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=desc) into [master](https://codecov.io/gh/goreleaser/goreleaser/commit/ce267acabf3205e889ead098a589d905bef32d01?src=pr&el=desc) will **decrease** coverage by `0.56%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/goreleaser/goreleaser/pull/871/graphs/tree.svg?width=650&token=3bJvv6b7hN&height=150&src=pr)](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #871      +/-   ##
==========================================
- Coverage   88.33%   87.77%   -0.57%     
==========================================
  Files          48       48              
  Lines        2340     2355      +15     
==========================================
  Hits         2067     2067              
- Misses        217      232      +15     
  Partials       56       56
```


| [Impacted Files](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/config/config.go](https://codecov.io/gh/goreleaser/goreleaser/pull/871/diff?src=pr&el=tree#diff-cGtnL2NvbmZpZy9jb25maWcuZ28=) | `100% <ø> (ø)` | :arrow_up: |
| [internal/client/github.go](https://codecov.io/gh/goreleaser/goreleaser/pull/871/diff?src=pr&el=tree#diff-aW50ZXJuYWwvY2xpZW50L2dpdGh1Yi5nbw==) | `5.4% <0%> (-0.85%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=footer). Last update [ce267ac...d87aa0f](https://codecov.io/gh/goreleaser/goreleaser/pull/871?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

caarlos0: we already use the masterminds/semver lib, and it already has a method to find out if the a version is a prerelease: https://godoc.org/github.com/Masterminds/semver#Version.Prerelease

we are already parsing the version in the git pipe I think... maybe we can make this even simpler?
caarlos0: oh, and thanks for the PR :D 
soupdiver: @caarlos0 Simplified the code. What do you think?
caarlos0: Looks good, but i think a better place for is in the release pipe... 

We could also have a `PreRelease` field on `context.Git`, so we can parse the tag a single time (maybe on the git pipe)...

What do you think?
soupdiver: I think I found a good place where to put the logic and also added a test.
Not sure about codecov results.
caarlos0: Hi @soupdiver 

one small comment, and after doing it we can probably increase the test coverage again :D 

Thanks again for the PR and all the effort 🚀
soupdiver: @caarlos0 I had a look at your suggestion. I put it into `github.go` because I thought about the case if some hoster does not provide pre-releases. That's why I put the logic into the specific implementation.
If we only support github this maybe doesn't make _that_ much of a difference.

Another thing:
Since `ctx.Config.Release.Prerelease` holds a string value (true, false or auto) we have to map this to a boolean at some point. I could put the logic of parsing and decision if we mark as pre-release into `release.go`. But then I would have to add another variable to context or release to hold this boolean. Otherwise I could add the boolean as a parameter to `CreateRelease` which would change the interface and cause need for changes in other places as well I think.

Both are possible. Which one do you prefer?
caarlos0: hmm, and there is another "problem", maybe, at some point, users will want to use the pre-release part of the tag on name templates... I guess we could either parse the tag multiple times or put all this in a single place...  anyway, we can leave this for later.

about the best place to put the decision, the main argument to put it in the release pipe `Default()` is that there is a pipe that writes the "effective config file" to dist (which I usually ask for on bug issues), so we will know before actually releasing what kind of release it will do... 

about other release providers that may not support pre-releases, I think we would just not pass that option to them in that case... anyway, it is a speculation, as now we don't have others haha 

all that being said, I would put that logic on the `release.Default` 🤔
?
soupdiver: Can I expected that `ctx.Git` is already populated when `pipe.Default` is run?
caarlos0: > Can I expected that `ctx.Git` is already populated when `pipe.Default` is run?

hmm, right now, no, but we should... let me fix that!
caarlos0: #879
soupdiver: @caarlos0 I took the existing tests as a template for mine but something goes wrong 🤔 
It seems the values set at
https://github.com/goreleaser/goreleaser/blob/0e0631a8d4616d4ec4c1ad66a62425ce5cfb273c/internal/pipe/release/release_test.go#L169
and
https://github.com/goreleaser/goreleaser/blob/0e0631a8d4616d4ec4c1ad66a62425ce5cfb273c/internal/pipe/release/release_test.go#L173

are not passed down the pipeline somehow.

See: https://travis-ci.org/goreleaser/goreleaser/builds/460234527#L712
and https://travis-ci.org/goreleaser/goreleaser/builds/460234527#L718

you have an idea what's wrong there?
caarlos0: I'll try to take a look later...
caarlos0: Oh, inside the release tests, the pipe that detects the git tag does not run, that's why it is not working.

you can cherry-pick this commit https://github.com/goreleaser/goreleaser/commit/fb8e5701814c3f7fbe23b5d7a60640d3cb74f46b and it will be fixed. 
soupdiver: check, will try

Patch:
diff --git a/internal/client/github.go b/internal/client/github.go
index 0cfcf4c4..93052b3b 100644
--- a/internal/client/github.go
+++ b/internal/client/github.go
@@ -94,12 +94,13 @@ func (c *githubClient) CreateRelease(ctx *context.Context, body string) (int64,
 	if err != nil {
 		return 0, err
 	}
+
 	var data = &github.RepositoryRelease{
 		Name:       github.String(title),
 		TagName:    github.String(ctx.Git.CurrentTag),
 		Body:       github.String(body),
 		Draft:      github.Bool(ctx.Config.Release.Draft),
-		Prerelease: github.Bool(ctx.Config.Release.Prerelease),
+		Prerelease: github.Bool(ctx.PreRelease),
 	}
 	release, _, err = c.client.Repositories.GetReleaseByTag(
 		ctx,
diff --git a/internal/pipe/release/release.go b/internal/pipe/release/release.go
index ef3b47ba..c5ae47a5 100644
--- a/internal/pipe/release/release.go
+++ b/internal/pipe/release/release.go
@@ -3,12 +3,14 @@ package release
 import (
 	"os"
 
+	"github.com/Masterminds/semver"
 	"github.com/apex/log"
 	"github.com/goreleaser/goreleaser/internal/artifact"
 	"github.com/goreleaser/goreleaser/internal/client"
 	"github.com/goreleaser/goreleaser/internal/pipe"
 	"github.com/goreleaser/goreleaser/internal/semerrgroup"
 	"github.com/goreleaser/goreleaser/pkg/context"
+	"github.com/pkg/errors"
 )
 
 // Pipe for github release
@@ -31,6 +33,28 @@ func (Pipe) Default(ctx *context.Context) error {
 		return err
 	}
 	ctx.Config.Release.GitHub = repo
+
+	ctx.PreRelease = false
+	// Check if we have to check the git tag for an indicator to mark as pre release
+	switch ctx.Config.Release.Prerelease {
+	case "auto":
+		sv, err := semver.NewVersion(ctx.Git.CurrentTag)
+		if err != nil {
+			return errors.Wrapf(err, "Failed to parse tag %s as semver", ctx.Git.CurrentTag)
+		}
+
+		if sv.Prerelease() != "" {
+			ctx.PreRelease = true
+		}
+		log.Debugf("Pre-Release was detected for tag %s: %v", ctx.Git.CurrentTag, ctx.PreRelease)
+	case "true":
+		ctx.PreRelease = true
+	case "false":
+		ctx.PreRelease = false
+	default:
+		log.Warnf("Invalid value %s for prerelease. Should be auto, true or false")
+	}
+
 	return nil
 }
 
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 9796f652..a3c8a6cb 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -158,8 +158,8 @@ type Archive struct {
 type Release struct {
 	GitHub       Repo   `yaml:",omitempty"`
 	Draft        bool   `yaml:",omitempty"`
-	Prerelease   bool   `yaml:",omitempty"`
 	Disable      bool   `yaml:",omitempty"`
+	Prerelease   string `yaml:",omitempty"`
 	NameTemplate string `yaml:"name_template,omitempty"`
 }
 
diff --git a/pkg/context/context.go b/pkg/context/context.go
index 38a4d07b..ce830c13 100644
--- a/pkg/context/context.go
+++ b/pkg/context/context.go
@@ -41,6 +41,7 @@ type Context struct {
 	SkipValidate bool
 	RmDist       bool
 	Debug        bool
+	PreRelease   bool
 	Parallelism  int
 }
 
diff --git a/www/content/release.md b/www/content/release.md
index 204a78f8..5f225dd3 100644
--- a/www/content/release.md
+++ b/www/content/release.md
@@ -24,9 +24,11 @@ release:
   # Default is false.
   draft: true
 
+  # If set to auto, will mark the release as not ready for production
+  # in case there is an indicator for this in the tag e.g. v1.0.0.-rc1
   # If set to true, will mark the release as not ready for production.
   # Default is false.
-  prerelease: true
+  prerelease: auto
 
   # You can change the name of the GitHub release.
   # Default is ``


Test patch:
diff --git a/internal/pipe/release/release_test.go b/internal/pipe/release/release_test.go
index e7bc2aa8..ca73bde9 100644
--- a/internal/pipe/release/release_test.go
+++ b/internal/pipe/release/release_test.go
@@ -149,6 +149,35 @@ func TestDefault(t *testing.T) {
 	assert.Equal(t, "goreleaser", ctx.Config.Release.GitHub.Owner)
 }
 
+func TestDefaultPreReleaseAuto(t *testing.T) {
+	_, back := testlib.Mktmp(t)
+	defer back()
+	testlib.GitInit(t)
+	testlib.GitRemoteAdd(t, "git@github.com:goreleaser/goreleaser.git")
+
+	t.Run("auto-release", func(t *testing.T) {
+		var ctx = context.New(config.Project{
+			Release: config.Release{
+				Prerelease: "auto",
+			},
+		})
+		ctx.Git.CurrentTag = "v1.0.0"
+		assert.NoError(t, Pipe{}.Default(ctx))
+		assert.Equal(t, false, ctx.PreRelease)
+	})
+
+	t.Run("auto-rc", func(t *testing.T) {
+		var ctx = context.New(config.Project{
+			Release: config.Release{
+				Prerelease: "auto",
+			},
+		})
+		ctx.Git.CurrentTag = "v1.0.1-rc1"
+		assert.NoError(t, Pipe{}.Default(ctx))
+		assert.Equal(t, true, ctx.PreRelease)
+	})
+}
+
 func TestDefaultPipeDisabled(t *testing.T) {
 	_, back := testlib.Mktmp(t)
 	defer back()