chore: replace logger and debuglogger with apex/log

This should hopefully lead to a nicer UX and output. Closes #331
aevea · Mar 7, 2021 · 59e29b9 · 59e29b9
1 parent 0055928
commit 59e29b9
Show file tree

Hide file tree

Showing 23 changed files with 197 additions and 184 deletions.
diff --git a/go.mod b/go.mod
@@ -1,21 +1,29 @@
 module github.com/aevea/commitsar
 
-go 1.13
+go 1.16
 
 require (
-	github.com/aevea/git/v2 v2.3.1
+	github.com/Microsoft/go-winio v0.4.16 // indirect
+	github.com/aevea/git/v3 v3.0.0
 	github.com/aevea/integrations v0.5.0
 	github.com/aevea/magefiles v0.0.0-20200424121010-0004d5a7a2fe
 	github.com/aevea/quoad v0.4.0
+	github.com/apex/log v1.9.0
 	github.com/go-git/go-git/v5 v5.2.0
 	github.com/go-openapi/strfmt v0.19.5 // indirect
 	github.com/google/go-github/v32 v32.1.0
+	github.com/imdario/mergo v0.3.11 // indirect
 	github.com/jedib0t/go-pretty v4.3.0+incompatible
+	github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351 // indirect
 	github.com/logrusorgru/aurora v2.0.3+incompatible
 	github.com/magefile/mage v1.11.0
 	github.com/mattn/go-runewidth v0.0.9 // indirect
 	github.com/spf13/cobra v1.1.3
 	github.com/spf13/viper v1.7.1
 	github.com/stretchr/testify v1.7.0
+	github.com/xanzy/ssh-agent v0.3.0 // indirect
+	golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 // indirect
+	golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 // indirect
 	golang.org/x/oauth2 v0.0.0-20210220000619-9bb904979d93
+	golang.org/x/sys v0.0.0-20210305230114-8fe3ee5dd75b // indirect
 )
diff --git a/go.sum b/go.sum
diff --git a/internal/commitpipeline/commits_between_branches.go b/internal/commitpipeline/commits_between_branches.go
@@ -1,7 +1,7 @@
 package commitpipeline
 
 import (
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 	"github.com/go-git/go-git/v5/plumbing"
 )
 

diff --git a/internal/commitpipeline/commits_between_branches_test.go b/internal/commitpipeline/commits_between_branches_test.go
@@ -3,12 +3,12 @@ package commitpipeline
 import (
 	"testing"
 
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestAllCommits(t *testing.T) {
-	gitRepo, err := history.OpenGit("../../testdata/long-history", nil)
+	gitRepo, err := history.OpenGit("../../testdata/long-history")
 
 	assert.NoError(t, err)
 
@@ -19,7 +19,7 @@ func TestAllCommits(t *testing.T) {
 		AllCommits:     true,
 	}
 
-	pipeline, err := New(nil, nil, &options)
+	pipeline, err := New(&options)
 
 	assert.NoError(t, err)
 
@@ -40,7 +40,7 @@ func TestAllCommits(t *testing.T) {
 }
 
 func TestLimitCommits(t *testing.T) {
-	gitRepo, err := history.OpenGit("../../testdata/long-history", nil)
+	gitRepo, err := history.OpenGit("../../testdata/long-history")
 
 	assert.NoError(t, err)
 
@@ -51,7 +51,7 @@ func TestLimitCommits(t *testing.T) {
 		AllCommits:     false,
 	}
 
-	pipeline, err := New(nil, nil, &options)
+	pipeline, err := New(&options)
 
 	assert.NoError(t, err)
 

diff --git a/internal/commitpipeline/commits_between_hashes.go b/internal/commitpipeline/commits_between_hashes.go
@@ -3,7 +3,7 @@ package commitpipeline
 import (
 	"strings"
 
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 	"github.com/go-git/go-git/v5/plumbing"
 )
 

diff --git a/internal/commitpipeline/commits_between_hashes_test.go b/internal/commitpipeline/commits_between_hashes_test.go
@@ -3,12 +3,12 @@ package commitpipeline
 import (
 	"testing"
 
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestCommitsBetweenHashes(t *testing.T) {
-	gitRepo, err := history.OpenGit("../../testdata/commits-on-different-branches", nil)
+	gitRepo, err := history.OpenGit("../../testdata/commits-on-different-branches")
 
 	assert.NoError(t, err)
 
@@ -25,7 +25,7 @@ func TestCommitsBetweenHashes(t *testing.T) {
 }
 
 func TestCommitsBetweenHashesOnlyTo(t *testing.T) {
-	gitRepo, err := history.OpenGit("../../testdata/commits-on-different-branches", nil)
+	gitRepo, err := history.OpenGit("../../testdata/commits-on-different-branches")
 
 	assert.NoError(t, err)
 

diff --git a/internal/commitpipeline/identify_same_branch.go b/internal/commitpipeline/identify_same_branch.go
@@ -1,7 +1,7 @@
 package commitpipeline
 
 import (
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 )
 
 // IdentifySameBranch breaks up the reference names and tries to identify if the branches are the same

diff --git a/internal/commitpipeline/log_branch.go b/internal/commitpipeline/log_branch.go
@@ -1,6 +1,9 @@
 package commitpipeline
 
-import history "github.com/aevea/git/v2"
+import (
+	history "github.com/aevea/git/v3"
+	"github.com/apex/log"
+)
 
 // logBranch outputs the branch which is being checked into the console
 func (pipeline *Pipeline) logBranch(gitRepo *history.Git) error {
@@ -10,7 +13,7 @@ func (pipeline *Pipeline) logBranch(gitRepo *history.Git) error {
 		return err
 	}
 
-	pipeline.Logger.Printf("Starting analysis of commits on branch %s", branch.Name())
+	log.Infof("Starting analysis of commits on branch %s", branch.Name())
 
 	return nil
 }
diff --git a/internal/commitpipeline/log_branch_test.go b/internal/commitpipeline/log_branch_test.go
@@ -1,30 +1,29 @@
 package commitpipeline
 
 import (
-	"bytes"
-	"log"
 	"testing"
 
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
+	"github.com/apex/log"
+	"github.com/apex/log/handlers/memory"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestLogBranch(t *testing.T) {
-	var testString bytes.Buffer
+	handler := memory.New()
 
-	testLogger := log.Logger{}
-	testLogger.SetOutput(&testString)
+	log.SetHandler(handler)
 
-	runner, err := New(&testLogger, nil, nil)
+	runner, err := New(nil)
 
 	assert.NoError(t, err)
 
-	gitRepo, err := history.OpenGit("../../testdata/commits-on-master", nil)
+	gitRepo, err := history.OpenGit("../../testdata/commits-on-master")
 
 	assert.NoError(t, err)
 
 	err = runner.logBranch(gitRepo)
 
 	assert.NoError(t, err)
-	assert.Equal(t, "Starting analysis of commits on branch refs/heads/master\n", testString.String())
+	assert.Equal(t, "Starting analysis of commits on branch refs/heads/master", handler.Entries[0].Message)
 }
diff --git a/internal/commitpipeline/pipeline.go b/internal/commitpipeline/pipeline.go
@@ -1,16 +1,8 @@
 package commitpipeline
 
-import (
-	"io/ioutil"
-	"log"
-	"os"
-)
-
 type Pipeline struct {
-	Logger      *log.Logger
-	DebugLogger *log.Logger
-	args        []string
-	options     Options
+	args    []string
+	options Options
 }
 
 type Options struct {
@@ -27,14 +19,7 @@ type Options struct {
 	RequiredScopes []string
 }
 
-func New(logger, debugLogger *log.Logger, options *Options, args ...string) (*Pipeline, error) {
-	if logger == nil {
-		logger = log.New(os.Stdout, "", 0)
-	}
-	if debugLogger == nil {
-		debugLogger = log.New(ioutil.Discard, "[DEBUG] ", 0)
-	}
-
+func New(options *Options, args ...string) (*Pipeline, error) {
 	if options == nil {
 		options = &Options{
 			Path:           ".",
@@ -46,10 +31,8 @@ func New(logger, debugLogger *log.Logger, options *Options, args ...string) (*Pi
 	}
 
 	return &Pipeline{
-		Logger:      logger,
-		DebugLogger: debugLogger,
-		options:     *options,
-		args:        args,
+		options: *options,
+		args:    args,
 	}, nil
 }
 

diff --git a/internal/commitpipeline/run.go b/internal/commitpipeline/run.go
@@ -6,14 +6,15 @@ import (
 
 	"github.com/aevea/commitsar/internal/dispatcher"
 	"github.com/aevea/commitsar/pkg/text"
-	history "github.com/aevea/git/v2"
+	history "github.com/aevea/git/v3"
 	"github.com/aevea/quoad"
+	"github.com/apex/log"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/logrusorgru/aurora"
 )
 
 func (pipeline *Pipeline) Run() (*dispatcher.PipelineSuccess, error) {
-	gitRepo, err := history.OpenGit(pipeline.options.Path, pipeline.DebugLogger)
+	gitRepo, err := history.OpenGit(pipeline.options.Path)
 
 	if err != nil {
 		return nil, err
@@ -56,15 +57,15 @@ func (pipeline *Pipeline) Run() (*dispatcher.PipelineSuccess, error) {
 
 		parsedCommit := quoad.ParseCommitMessage(commitObject.Message)
 
-		pipeline.DebugLogger.Printf("Commit found: [hash] %v [message] %v \n", parsedCommit.Hash.String(), parsedCommit.Heading)
+		log.Debugf("Commit found: [hash] %v [message] %v", parsedCommit.Hash.String(), parsedCommit.Heading)
 
 		if !text.IsMergeCommit(commitObject.Message) && !text.IsInitialCommit(commitObject.Message) {
 			filteredCommits = append(filteredCommits, commitHash)
 		}
 	}
 
-	pipeline.Logger.Printf("\n%v commits filtered out\n", len(commits)-len(filteredCommits))
-	pipeline.Logger.Printf("\nFound %v commit to check\n", len(filteredCommits))
+	log.Infof("%v commits filtered out", len(commits)-len(filteredCommits))
+	log.Infof("Found %v commit to check", len(filteredCommits))
 
 	if len(filteredCommits) == 0 {
 		return nil, errors.New(aurora.Red("No commits found, please check you are on a branch outside of main").String())
@@ -102,16 +103,16 @@ func (pipeline *Pipeline) Run() (*dispatcher.PipelineSuccess, error) {
 		failingCommitTable.SetOutputMirror(os.Stdout)
 		failingCommitTable.Render()
 
-		pipeline.Logger.Printf("%v of %v commits are not conventional commit compliant\n", aurora.Red(len(faultyCommits)), aurora.Red(len(commits)))
+		log.Infof("%v of %v commits are not conventional commit compliant", aurora.Red(len(faultyCommits)), aurora.Red(len(commits)))
 
-		pipeline.Logger.Print("\nExpected format is for example:      chore(ci): this is a test\n")
-		pipeline.Logger.Print("Please see https://www.conventionalcommits.org for help on how to structure commits\n\n")
+		log.Info("Expected format is for example:      chore(ci): this is a test")
+		log.Info("Please see https://www.conventionalcommits.org for help on how to structure commits")
 
 		return nil, errors.New(aurora.Red("Not all commits are conventional commits, please check the commits listed above").String())
 	}
 
 	return &dispatcher.PipelineSuccess{
-		Message:      aurora.Sprintf(aurora.Green("All %v commits are conventional commit compliant\n"), len(filteredCommits)),
+		Message:      aurora.Sprintf(aurora.Green("All %v commits are conventional commit compliant"), len(filteredCommits)),
 		PipelineName: pipeline.Name(),
 	}, nil
 }
diff --git a/internal/dispatcher/dispatcher.go b/internal/dispatcher/dispatcher.go
@@ -1,16 +1,13 @@
 package dispatcher
 
 import (
-	"io/ioutil"
-	"log"
 	"runtime"
 	"sync"
 )
 
 // Dispatcher is the central place which runs Pipelines. maxWorkers is based by default on number of CPUs * 2 accounting for modern CPU architectures.
 type Dispatcher struct {
-	debugLogger *log.Logger
-	maxWorkers  int
+	maxWorkers int
 }
 
 // Results contains the aggregated results of both the succesful and error pipelines.
@@ -20,12 +17,9 @@ type Results struct {
 }
 
 // New returns a set up instance of Dispatcher
-func New(debugLogger *log.Logger) *Dispatcher {
-	if debugLogger == nil {
-		debugLogger = log.New(ioutil.Discard, "", 0)
-	}
+func New() *Dispatcher {
 
-	return &Dispatcher{debugLogger: debugLogger, maxWorkers: runtime.NumCPU() * 2}
+	return &Dispatcher{maxWorkers: runtime.NumCPU() * 2}
 }
 
 // RunPipelines will run asynchronously all pipelines passed to it. It is limited only by the maxWorkers field on Dispatcher.

diff --git a/internal/dispatcher/dispatcher_test.go b/internal/dispatcher/dispatcher_test.go
@@ -8,7 +8,7 @@ import (
 )
 
 func TestDispatcher(t *testing.T) {
-	dispatcher := New(nil)
+	dispatcher := New()
 	dispatcher.maxWorkers = 1
 
 	pipelines := []Pipeliner{TestPipeline{TestName: "pipeline1", TestFn: successPipeline}}

diff --git a/internal/dispatcher/handle_errors.go b/internal/dispatcher/handle_errors.go
@@ -1,6 +1,10 @@
 package dispatcher
 
-import "sync"
+import (
+	"sync"
+
+	"github.com/apex/log"
+)
 
 func (dispatch *Dispatcher) handleErrors(
 	wg *sync.WaitGroup,
@@ -10,7 +14,7 @@ func (dispatch *Dispatcher) handleErrors(
 	defer wg.Done()
 
 	for message := range channel {
-		dispatch.debugLogger.Printf("[%s] %s", message.PipelineName, message.Error)
+		log.Debugf("[%s] %s", message.PipelineName, message.Error)
 
 		results.Errors = append(results.Errors, message)
 	}

diff --git a/internal/dispatcher/success_handler.go b/internal/dispatcher/success_handler.go
@@ -1,6 +1,10 @@
 package dispatcher
 
-import "sync"
+import (
+	"sync"
+
+	"github.com/apex/log"
+)
 
 func (dispatch *Dispatcher) handleSuccess(
 	wg *sync.WaitGroup,
@@ -10,7 +14,7 @@ func (dispatch *Dispatcher) handleSuccess(
 	defer wg.Done()
 
 	for message := range channel {
-		dispatch.debugLogger.Printf("[%s] %s", message.PipelineName, message.Message)
+		log.Debugf("[%s] %s", message.PipelineName, message.Message)
 
 		results.SuccessfulPipelines = append(results.SuccessfulPipelines, message)
 	}