Repository: lindell/multi-gitter
Pull request #148 (2021-06-18 09:40:42)

refactor: moved stack trace getter


Patch:
diff --git a/internal/multigitter/run.go b/internal/multigitter/run.go
index 06cf782..853a93e 100755
--- a/internal/multigitter/run.go
+++ b/internal/multigitter/run.go
@@ -72,17 +72,15 @@ func (pr dryRunPullRequest) String() string {
 	return fmt.Sprintf("%s #0", pr.Repository.FullName())
 }
 
-type stackTracer interface {
-	StackTrace() errors.StackTrace
-}
-
 // Run runs a script for multiple repositories and creates PRs with the changes made
 func (r Runner) Run(ctx context.Context) error {
+	// Fetch all repositories that are are going to be used in the run
 	repos, err := r.VersionController.GetRepositories(ctx)
 	if err != nil {
 		return errors.Wrap(err, "could not fetch repositories")
 	}
 
+	// Setting up a "counter" that keeps track of successful and failed runs
 	rc := repocounter.NewCounter()
 	defer func() {
 		if info := rc.Info(); info != "" {
@@ -108,15 +106,13 @@ func (r Runner) Run(ctx context.Context) error {
 				logger.Info(err)
 			}
 			rc.AddError(err, repos[i])
+
 			if log.IsLevelEnabled(log.TraceLevel) {
-				if err, ok := err.(stackTracer); ok {
-					trace := ""
-					for _, f := range err.StackTrace() {
-						trace += fmt.Sprintf("%+s:%d\n", f, f)
-					}
-					log.Trace(trace)
+				if stackTrace := getStackTrace(err); stackTrace != "" {
+					log.Trace(stackTrace)
 				}
 			}
+
 			return
 		}
 
@@ -197,6 +193,7 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) (doma
 		fmt.Sprintf("REPOSITORY=%s", repo.FullName()),
 	)
 
+	// Setup logger that transfers stdout and stderr from the run to logs
 	writer := logger.NewLogger(log)
 	defer writer.Close()
 	cmd.Stdout = writer
@@ -251,6 +248,7 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) (doma
 		}
 	}
 
+	log.Info("Pushing changes to remote")
 	err = sourceController.Push(remoteName)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not push changes")
@@ -260,7 +258,7 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) (doma
 		return nil, nil
 	}
 
-	log.Info("Change done, creating pull request")
+	log.Info("Creating pull request")
 	pr, err := r.VersionController.CreatePullRequest(ctx, repo, prRepo, domain.NewPullRequest{
 		Title:     r.PullRequestTitle,
 		Body:      r.PullRequestBody,
diff --git a/internal/multigitter/shared.go b/internal/multigitter/shared.go
index 8032f89..30b1c05 100644
--- a/internal/multigitter/shared.go
+++ b/internal/multigitter/shared.go
@@ -1,6 +1,7 @@
 package multigitter
 
 import (
+	"fmt"
 	"syscall"
 
 	"github.com/lindell/multi-gitter/internal/domain"
@@ -31,3 +32,18 @@ type Git interface {
 	Push(remoteName string) error
 	AddRemote(name, url string) error
 }
+
+type stackTracer interface {
+	StackTrace() errors.StackTrace
+}
+
+func getStackTrace(err error) string {
+	if err, ok := err.(stackTracer); ok {
+		trace := ""
+		for _, f := range err.StackTrace() {
+			trace += fmt.Sprintf("%+s:%d\n", f, f)
+		}
+		return trace
+	}
+	return ""
+}


Test patch:
diff --git a/tests/table_test.go b/tests/table_test.go
index b7d1122..1db1d25 100644
--- a/tests/table_test.go
+++ b/tests/table_test.go
@@ -81,7 +81,8 @@ func TestTable(t *testing.T) {
 
 				assert.Contains(t, runData.logOut, "Running on 1 repositories")
 				assert.Contains(t, runData.logOut, "Cloning and running script")
-				assert.Contains(t, runData.logOut, "Change done, creating pull request")
+				assert.Contains(t, runData.logOut, "Pushing changes to remote")
+				assert.Contains(t, runData.logOut, "Creating pull request")
 
 				assert.Equal(t, `Repositories with a successful run:
   owner/should-change #1
@@ -113,7 +114,8 @@ func TestTable(t *testing.T) {
 
 				assert.Contains(t, runData.logOut, "Running on 1 repositories")
 				assert.Contains(t, runData.logOut, "Cloning and running script")
-				assert.Contains(t, runData.logOut, "Change done, creating pull request")
+				assert.Contains(t, runData.logOut, "Pushing changes to remote")
+				assert.Contains(t, runData.logOut, "Creating pull request")
 
 				assert.Equal(t, `Repositories with a successful run:
   owner/should-change #1
@@ -458,7 +460,8 @@ Repositories with a successful run:
 
 				assert.Contains(t, runData.logOut, "Running on 1 repositories")
 				assert.Contains(t, runData.logOut, "Cloning and running script")
-				assert.Contains(t, runData.logOut, "Change done, creating pull request")
+				assert.Contains(t, runData.logOut, "Pushing changes to remote")
+				assert.Contains(t, runData.logOut, "Creating pull request")
 
 				assert.Equal(t, `Repositories with a successful run:
   owner/should-change #1
@@ -549,7 +552,8 @@ Repositories with a successful run:
 				assert.Contains(t, runData.logOut, "Running on 1 repositories")
 				assert.Contains(t, runData.logOut, "Cloning and running script")
 				assert.Contains(t, runData.logOut, "Forking repository")
-				assert.Contains(t, runData.logOut, "Change done, creating pull request")
+				assert.Contains(t, runData.logOut, "Pushing changes to remote")
+				assert.Contains(t, runData.logOut, "Creating pull request")
 
 				assert.Equal(t, `Repositories with a successful run:
   owner/should-change #1
@@ -589,7 +593,8 @@ Repositories with a successful run:
 				assert.Contains(t, runData.logOut, "Running on 1 repositories")
 				assert.Contains(t, runData.logOut, "Cloning and running script")
 				assert.Contains(t, runData.logOut, "Forking repository")
-				assert.Contains(t, runData.logOut, "Change done, creating pull request")
+				assert.Contains(t, runData.logOut, "Pushing changes to remote")
+				assert.Contains(t, runData.logOut, "Creating pull request")
 
 				assert.Equal(t, `Repositories with a successful run:
   owner/should-change #1