refactor/fix: improved CLI (#937)

* refactor: added middleware for action logs/error handling

* refactor: moved custom changelog load from main.go

* fix/refactor: CLI improvements

* test: do not pollute ./dist

internal/middleware/doc.go

@@ -0,0 +1,2 @@
+// Package middleware define middlewares for Actions.
+package middleware

internal/middleware/error.go

@@ -0,0 +1,23 @@
+package middleware
+
+import (
+	"github.com/apex/log"
+	"github.com/goreleaser/goreleaser/internal/pipe"
+	"github.com/goreleaser/goreleaser/pkg/context"
+)
+
+// ErrHandler handles an action error, ignoring and logging pipe skipped
+// errors.
+func ErrHandler(action Action) Action {
+	return func(ctx *context.Context) error {
+		var err = action(ctx)
+		if err == nil {
+			return nil
+		}
+		if pipe.IsSkip(err) {
+			log.WithError(err).Warn("pipe skipped")
+			return nil
+		}
+		return err
+	}
+}

internal/middleware/error_test.go

@@ -0,0 +1,23 @@
+package middleware
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/goreleaser/goreleaser/internal/pipe"
+	"github.com/stretchr/testify/require"
+)
+
+func TestError(t *testing.T) {
+	t.Run("no errors", func(t *testing.T) {
+		require.NoError(t, ErrHandler(mockAction(nil))(ctx))
+	})
+
+	t.Run("pipe skipped", func(t *testing.T) {
+		require.NoError(t, ErrHandler(mockAction(pipe.ErrSkipValidateEnabled))(ctx))
+	})
+
+	t.Run("some err", func(t *testing.T) {
+		require.Error(t, ErrHandler(mockAction(fmt.Errorf("pipe errored")))(ctx))
+	})
+}

internal/middleware/logging.go

@@ -0,0 +1,37 @@
+package middleware
+
+import (
+	"strings"
+
+	"github.com/apex/log"
+	"github.com/apex/log/handlers/cli"
+	"github.com/fatih/color"
+	"github.com/goreleaser/goreleaser/pkg/context"
+)
+
+// Padding is a logging initial padding.
+type Padding int
+
+// DefaultInitialPadding is the default padding in the log library.
+const DefaultInitialPadding Padding = 3
+
+// ExtraPadding is the double of the DefaultInitialPadding.
+const ExtraPadding Padding = DefaultInitialPadding * 2
+
+// Logging pretty prints the given action and its title.
+// You can have different padding levels by providing different initial
+// paddings. The middleware will print the title in the given padding and the
+// action logs in padding+default padding.
+// The default padding in the log library is 3.
+// The middleware always resets to the default padding.
+func Logging(title string, next Action, padding Padding) Action {
+	return func(ctx *context.Context) error {
+		defer func() {
+			cli.Default.Padding = int(DefaultInitialPadding)
+		}()
+		cli.Default.Padding = int(padding)
+		log.Infof(color.New(color.Bold).Sprint(strings.ToUpper(title)))
+		cli.Default.Padding = int(padding + DefaultInitialPadding)
+		return next(ctx)
+	}
+}

internal/middleware/logging_test.go

@@ -0,0 +1,11 @@
+package middleware
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestLogging(t *testing.T) {
+	require.NoError(t, Logging("foo", mockAction(nil), DefaultInitialPadding)(ctx))
+}

internal/middleware/middleware.go

@@ -0,0 +1,8 @@
+package middleware
+
+import "github.com/goreleaser/goreleaser/pkg/context"
+
+// Action is a function that takes a context and returns an error.
+// It is is used on Pipers, Defaulters and Publishers, although they are not
+// aware of this generalization.
+type Action func(ctx *context.Context) error

internal/middleware/middleware_test.go

@@ -0,0 +1,11 @@
+package middleware
+
+import "github.com/goreleaser/goreleaser/pkg/context"
+
+var ctx = &context.Context{}
+
+func mockAction(err error) Action {
+	return func(ctx *context.Context) error {
+		return err
+	}
+}

internal/pipe/changelog/changelog.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/apex/log"
-
 	"github.com/goreleaser/goreleaser/internal/git"
 	"github.com/goreleaser/goreleaser/internal/pipe"
 	"github.com/goreleaser/goreleaser/pkg/context"
@@ -32,8 +31,15 @@ func (Pipe) Run(ctx *context.Context) error {
 	if ctx.Config.Changelog.Skip {
 		return pipe.Skip("changelog should not be built")
 	}
+	// TODO: should probably have a different field for the filename and its
+	// contents.
 	if ctx.ReleaseNotes != "" {
-		return pipe.Skip("release notes already provided via --release-notes")
+		notes, err := loadFromFile(ctx.ReleaseNotes)
+		if err != nil {
+			return err
+		}
+		ctx.ReleaseNotes = notes
+		return nil
 	}
 	if ctx.Snapshot {
 		return pipe.Skip("not available for snapshots")
@@ -51,6 +57,16 @@ func (Pipe) Run(ctx *context.Context) error {
 	return ioutil.WriteFile(path, []byte(ctx.ReleaseNotes), 0644)
 }
 
+func loadFromFile(file string) (string, error) {
+	bts, err := ioutil.ReadFile(file)
+	if err != nil {
+		return "", err
+	}
+	log.WithField("file", file).Info("loaded custom release notes")
+	log.WithField("file", file).Debugf("custom release notes: \n%s", string(bts))
+	return string(bts), nil
+}
+
 func checkSortDirection(mode string) error {
 	switch mode {
 	case "":

internal/pipe/changelog/changelog_test.go

@@ -5,7 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/goreleaser/goreleaser/internal/testlib"
 	"github.com/goreleaser/goreleaser/pkg/config"
@@ -13,13 +13,20 @@ import (
 )
 
 func TestDescription(t *testing.T) {
-	assert.NotEmpty(t, Pipe{}.String())
+	require.NotEmpty(t, Pipe{}.String())
 }
 
 func TestChangelogProvidedViaFlag(t *testing.T) {
 	var ctx = context.New(config.Project{})
-	ctx.ReleaseNotes = "c0ff33 foo bar"
+	ctx.ReleaseNotes = "testdata/changes.md"
-	testlib.AssertSkipped(t, Pipe{}.Run(ctx))
+	require.NoError(t, Pipe{}.Run(ctx))
+	require.Equal(t, "c0ff33 coffeee\n", ctx.ReleaseNotes)
+}
+
+func TestChangelogProvidedViaFlagDoesntExist(t *testing.T) {
+	var ctx = context.New(config.Project{})
+	ctx.ReleaseNotes = "testdata/changes.nope"
+	require.EqualError(t, Pipe{}.Run(ctx), "open testdata/changes.nope: no such file or directory")
 }
 
 func TestChangelogSkip(t *testing.T) {
@@ -63,19 +70,19 @@ func TestChangelog(t *testing.T) {
 		},
 	})
 	ctx.Git.CurrentTag = "v0.0.2"
-	assert.NoError(t, Pipe{}.Run(ctx))
+	require.NoError(t, Pipe{}.Run(ctx))
-	assert.Contains(t, ctx.ReleaseNotes, "## Changelog")
+	require.Contains(t, ctx.ReleaseNotes, "## Changelog")
-	assert.NotContains(t, ctx.ReleaseNotes, "first")
+	require.NotContains(t, ctx.ReleaseNotes, "first")
-	assert.Contains(t, ctx.ReleaseNotes, "added feature 1")
+	require.Contains(t, ctx.ReleaseNotes, "added feature 1")
-	assert.Contains(t, ctx.ReleaseNotes, "fixed bug 2")
+	require.Contains(t, ctx.ReleaseNotes, "fixed bug 2")
-	assert.NotContains(t, ctx.ReleaseNotes, "docs")
+	require.NotContains(t, ctx.ReleaseNotes, "docs")
-	assert.NotContains(t, ctx.ReleaseNotes, "ignored")
+	require.NotContains(t, ctx.ReleaseNotes, "ignored")
-	assert.NotContains(t, ctx.ReleaseNotes, "cArs")
+	require.NotContains(t, ctx.ReleaseNotes, "cArs")
-	assert.NotContains(t, ctx.ReleaseNotes, "from goreleaser/some-branch")
+	require.NotContains(t, ctx.ReleaseNotes, "from goreleaser/some-branch")
 
 	bts, err := ioutil.ReadFile(filepath.Join(folder, "CHANGELOG.md"))
-	assert.NoError(t, err)
+	require.NoError(t, err)
-	assert.NotEmpty(t, string(bts))
+	require.NotEmpty(t, string(bts))
 }
 
 func TestChangelogSort(t *testing.T) {
@@ -125,14 +132,14 @@ func TestChangelogSort(t *testing.T) {
 		t.Run("changelog sort='"+cfg.Sort+"'", func(t *testing.T) {
 			ctx.Config.Changelog.Sort = cfg.Sort
 			entries, err := buildChangelog(ctx)
-			assert.NoError(t, err)
+			require.NoError(t, err)
-			assert.Len(t, entries, len(cfg.Entries))
+			require.Len(t, entries, len(cfg.Entries))
 			var changes []string
 			for _, line := range entries {
 				_, msg := extractCommitInfo(line)
 				changes = append(changes, msg)
 			}
-			assert.EqualValues(t, cfg.Entries, changes)
+			require.EqualValues(t, cfg.Entries, changes)
 		})
 	}
 }
@@ -143,7 +150,7 @@ func TestChangelogInvalidSort(t *testing.T) {
 			Sort: "dope",
 		},
 	})
-	assert.EqualError(t, Pipe{}.Run(ctx), ErrInvalidSortDirection.Error())
+	require.EqualError(t, Pipe{}.Run(ctx), ErrInvalidSortDirection.Error())
 }
 
 func TestChangelogOfFirstRelease(t *testing.T) {
@@ -162,10 +169,10 @@ func TestChangelogOfFirstRelease(t *testing.T) {
 	testlib.GitTag(t, "v0.0.1")
 	var ctx = context.New(config.Project{})
 	ctx.Git.CurrentTag = "v0.0.1"
-	assert.NoError(t, Pipe{}.Run(ctx))
+	require.NoError(t, Pipe{}.Run(ctx))
-	assert.Contains(t, ctx.ReleaseNotes, "## Changelog")
+	require.Contains(t, ctx.ReleaseNotes, "## Changelog")
 	for _, msg := range msgs {
-		assert.Contains(t, ctx.ReleaseNotes, msg)
+		require.Contains(t, ctx.ReleaseNotes, msg)
 	}
 }
 
@@ -187,7 +194,7 @@ func TestChangelogFilterInvalidRegex(t *testing.T) {
 		},
 	})
 	ctx.Git.CurrentTag = "v0.0.4"
-	assert.EqualError(t, Pipe{}.Run(ctx), "error parsing regexp: invalid or unsupported Perl syntax: `(?ia`")
+	require.EqualError(t, Pipe{}.Run(ctx), "error parsing regexp: invalid or unsupported Perl syntax: `(?ia`")
 }
 
 func TestChangelogNoTags(t *testing.T) {
@@ -196,8 +203,8 @@ func TestChangelogNoTags(t *testing.T) {
 	testlib.GitInit(t)
 	testlib.GitCommit(t, "first")
 	var ctx = context.New(config.Project{})
-	assert.Error(t, Pipe{}.Run(ctx))
+	require.Error(t, Pipe{}.Run(ctx))
-	assert.Empty(t, ctx.ReleaseNotes)
+	require.Empty(t, ctx.ReleaseNotes)
 }
 
 func TestChangelogOnBranchWithSameNameAsTag(t *testing.T) {
@@ -217,9 +224,9 @@ func TestChangelogOnBranchWithSameNameAsTag(t *testing.T) {
 	testlib.GitCheckoutBranch(t, "v0.0.1")
 	var ctx = context.New(config.Project{})
 	ctx.Git.CurrentTag = "v0.0.1"
-	assert.NoError(t, Pipe{}.Run(ctx))
+	require.NoError(t, Pipe{}.Run(ctx))
-	assert.Contains(t, ctx.ReleaseNotes, "## Changelog")
+	require.Contains(t, ctx.ReleaseNotes, "## Changelog")
 	for _, msg := range msgs {
-		assert.Contains(t, ctx.ReleaseNotes, msg)
+		require.Contains(t, ctx.ReleaseNotes, msg)
 	}
 }

internal/pipe/changelog/testdata/changes.md

@@ -0,0 +1 @@
+c0ff33 coffeee

internal/pipe/defaults/defaults.go

@@ -3,7 +3,7 @@
 package defaults
 
 import (
-	"github.com/apex/log"
+	"github.com/goreleaser/goreleaser/internal/middleware"
 	"github.com/goreleaser/goreleaser/pkg/context"
 	"github.com/goreleaser/goreleaser/pkg/defaults"
 )
@@ -24,8 +24,11 @@ func (Pipe) Run(ctx *context.Context) error {
 		ctx.Config.GitHubURLs.Download = "https://github.com"
 	}
 	for _, defaulter := range defaults.Defaulters {
-		log.Debug(defaulter.String())
+		if err := middleware.Logging(
-		if err := defaulter.Default(ctx); err != nil {
+			defaulter.String(),
+			middleware.ErrHandler(defaulter.Default),
+			middleware.ExtraPadding,
+		)(ctx); err != nil {
 			return err
 		}
 	}

internal/pipe/publish/publish.go

@@ -4,8 +4,7 @@ package publish
 import (
 	"fmt"
 
-	"github.com/apex/log"
+	"github.com/goreleaser/goreleaser/internal/middleware"
-	"github.com/fatih/color"
 	"github.com/goreleaser/goreleaser/internal/pipe"
 	"github.com/goreleaser/goreleaser/internal/pipe/artifactory"
 	"github.com/goreleaser/goreleaser/internal/pipe/brew"
@@ -54,23 +53,13 @@ func (Pipe) Run(ctx *context.Context) error {
 		return pipe.ErrSkipPublishEnabled
 	}
 	for _, publisher := range publishers {
-		log.Infof(color.New(color.Bold).Sprint(publisher.String()))
+		if err := middleware.Logging(
-		if err := handle(publisher.Publish(ctx)); err != nil {
+			publisher.String(),
+			middleware.ErrHandler(publisher.Publish),
+			middleware.ExtraPadding,
+		)(ctx); err != nil {
 			return errors.Wrapf(err, "%s: failed to publish artifacts", publisher.String())
 		}
 	}
 	return nil
 }
-
-// TODO: for now this is duplicated, we should have better error handling
-// eventually.
-func handle(err error) error {
-	if err == nil {
-		return nil
-	}
-	if pipe.IsSkip(err) {
-		log.WithField("reason", err.Error()).Warn("skipped")
-		return nil
-	}
-	return err
-}

main.go

@@ -4,14 +4,13 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/apex/log"
 	"github.com/apex/log/handlers/cli"
 	"github.com/caarlos0/ctrlc"
 	"github.com/fatih/color"
-	"github.com/goreleaser/goreleaser/internal/pipe"
+	"github.com/goreleaser/goreleaser/internal/middleware"
 	"github.com/goreleaser/goreleaser/internal/pipeline"
 	"github.com/goreleaser/goreleaser/internal/static"
 	"github.com/goreleaser/goreleaser/pkg/config"
@@ -34,7 +33,6 @@ type releaseOptions struct {
 	SkipSign     bool
 	SkipValidate bool
 	RmDist       bool
-	Debug        bool
 	Parallelism  int
 	Timeout      time.Duration
 }
@@ -56,11 +54,11 @@ func main() {
 	var config = releaseCmd.Flag("config", "Load configuration from file").Short('c').Short('f').PlaceHolder(".goreleaser.yml").String()
 	var releaseNotes = releaseCmd.Flag("release-notes", "Load custom release notes from a markdown file").PlaceHolder("notes.md").String()
 	var snapshot = releaseCmd.Flag("snapshot", "Generate an unversioned snapshot release, skipping all validations and without publishing any artifacts").Bool()
-	var skipPublish = releaseCmd.Flag("skip-publish", "Generates all artifacts but does not publish them anywhere").Bool()
+	var skipPublish = releaseCmd.Flag("skip-publish", "Skips publishing artifacts").Bool()
 	var skipSign = releaseCmd.Flag("skip-sign", "Skips signing the artifacts").Bool()
-	var skipValidate = releaseCmd.Flag("skip-validate", "Skips all git sanity checks").Bool()
+	var skipValidate = releaseCmd.Flag("skip-validate", "Skips several sanity checks").Bool()
 	var rmDist = releaseCmd.Flag("rm-dist", "Remove the dist folder before building").Bool()
-	var parallelism = releaseCmd.Flag("parallelism", "Amount of slow tasks to do in concurrently").Short('p').Default("4").Int() // TODO: use runtime.NumCPU here?
+	var parallelism = releaseCmd.Flag("parallelism", "Amount tasks to run concurrently").Short('p').Default("4").Int()
 	var timeout = releaseCmd.Flag("timeout", "Timeout to the entire release process").Default("30m").Duration()
 
 	app.Version(fmt.Sprintf("%v, commit %v, built at %v", version, commit, date))
@@ -93,7 +91,6 @@ func main() {
 			SkipSign:     *skipSign,
 			RmDist:       *rmDist,
 			Parallelism:  *parallelism,
-			Debug:        *debug,
 			Timeout:      *timeout,
 		}
 		if err := releaseProject(options); err != nil {
@@ -113,50 +110,25 @@ func releaseProject(options releaseOptions) error {
 	ctx, cancel := context.NewWithTimeout(cfg, options.Timeout)
 	defer cancel()
 	ctx.Parallelism = options.Parallelism
-	ctx.Debug = options.Debug
 	log.Debugf("parallelism: %v", ctx.Parallelism)
-	if options.ReleaseNotes != "" {
+	ctx.ReleaseNotes = options.ReleaseNotes
-		bts, err := ioutil.ReadFile(options.ReleaseNotes)
-		if err != nil {
-			return err
-		}
-		log.WithField("file", options.ReleaseNotes).Info("loaded custom release notes")
-		log.WithField("file", options.ReleaseNotes).Debugf("custom release notes: \n%s", string(bts))
-		ctx.ReleaseNotes = string(bts)
-	}
 	ctx.Snapshot = options.Snapshot
 	ctx.SkipPublish = ctx.Snapshot || options.SkipPublish
 	ctx.SkipValidate = ctx.Snapshot || options.SkipValidate
 	ctx.SkipSign = options.SkipSign
 	ctx.RmDist = options.RmDist
-	return doRelease(ctx)
+	return ctrlc.Default.Run(ctx, func() error {
-}
-
-func doRelease(ctx *context.Context) error {
-	defer func() { cli.Default.Padding = 3 }()
-	var release = func() error {
 		for _, pipe := range pipeline.Pipeline {
-			cli.Default.Padding = 3
+			if err := middleware.Logging(
-			log.Infof(color.New(color.Bold).Sprint(strings.ToUpper(pipe.String())))
+				pipe.String(),
-			cli.Default.Padding = 6
+				middleware.ErrHandler(pipe.Run),
-			if err := handle(pipe.Run(ctx)); err != nil {
+				middleware.DefaultInitialPadding,
+			)(ctx); err != nil {
 				return err
 			}
 		}
 		return nil
-	}
+	})
-	return ctrlc.Default.Run(ctx, release)
-}
-
-func handle(err error) error {
-	if err == nil {
-		return nil
-	}
-	if pipe.IsSkip(err) {
-		log.WithField("reason", err.Error()).Warn("skipped")
-		return nil
-	}
-	return err
 }
 
 // InitProject creates an example goreleaser.yml in the current directory
@@ -187,8 +159,8 @@ func loadConfig(path string) (config.Project, error) {
 		}
 		return proj, err
 	}
-	// the user didn't specified a config file and the known files
+	// the user didn't specified a config file and the known possible file names
-	// doest not exist, so, return an empty config and a nil err.
+	// don't exist, so, return an empty config and a nil err.
-	log.Warn("could not load config, using defaults")
+	log.Warn("could find a config file, using defaults...")
 	return config.Project{}, nil
 }

main_test.go

@@ -33,6 +33,8 @@ func TestReleaseProjectSkipPublish(t *testing.T) {
 }
 
 func TestConfigFileIsSetAndDontExist(t *testing.T) {
+	_, back := setup(t)
+	defer back()
 	params := testParams()
 	params.Config = "/this/wont/exist"
 	assert.Error(t, releaseProject(params))
@@ -71,6 +73,8 @@ func TestConfigFileDoesntExist(t *testing.T) {
 }
 
 func TestReleaseNotesFileDontExist(t *testing.T) {
+	_, back := setup(t)
+	defer back()
 	params := testParams()
 	params.ReleaseNotes = "/this/also/wont/exist"
 	assert.Error(t, releaseProject(params))
@@ -127,7 +131,6 @@ func TestInitProjectDefaultPipeFails(t *testing.T) {
 
 func testParams() releaseOptions {
 	return releaseOptions{
-		Debug:       true,
 		Parallelism: 4,
 		Snapshot:    true,
 		Timeout:     time.Minute,

pkg/context/context.go

@@ -40,7 +40,6 @@ type Context struct {
 	SkipSign     bool
 	SkipValidate bool
 	RmDist       bool
-	Debug        bool
 	PreRelease   bool
 	Parallelism  int
 	Semver       Semver