diff --git a/internal/multigitter/print.go b/internal/multigitter/print.go index 8be890f2..3aac1945 100755 --- a/internal/multigitter/print.go +++ b/internal/multigitter/print.go @@ -3,11 +3,12 @@ package multigitter import ( "context" "fmt" + "io" + "os" + "github.com/lindell/multi-gitter/internal/multigitter/repocounter" "github.com/lindell/multi-gitter/internal/scm" log "github.com/sirupsen/logrus" - "io" - "os" ) // Printer contains fields to be able to do the print command @@ -48,7 +49,7 @@ func (r Printer) Print(ctx context.Context) error { if err != errAborted { logger.Info(err) } - rc.AddError(err, repos[i]) + rc.AddError(err, repos[i], nil) return } diff --git a/internal/multigitter/repocounter/counter.go b/internal/multigitter/repocounter/counter.go index ea02556e..95c869bc 100644 --- a/internal/multigitter/repocounter/counter.go +++ b/internal/multigitter/repocounter/counter.go @@ -13,24 +13,32 @@ import ( type Counter struct { successPullRequests []scm.PullRequest successRepositories []scm.Repository - errorRepositories map[string][]scm.Repository + errors map[string][]errorInfo lock sync.RWMutex } +type errorInfo struct { + repository scm.Repository + pullRequest scm.PullRequest +} + // NewCounter create a new repo counter func NewCounter() *Counter { return &Counter{ - errorRepositories: map[string][]scm.Repository{}, + errors: map[string][]errorInfo{}, } } // AddError add a failing repository together with the error that caused it -func (r *Counter) AddError(err error, repo scm.Repository) { +func (r *Counter) AddError(err error, repo scm.Repository, pr scm.PullRequest) { defer r.lock.Unlock() r.lock.Lock() msg := err.Error() - r.errorRepositories[msg] = append(r.errorRepositories[msg], repo) + r.errors[msg] = append(r.errors[msg], errorInfo{ + repository: repo, + pullRequest: pr, + }) } // AddSuccessRepositories adds a repository that succeeded @@ -42,11 +50,11 @@ func (r *Counter) AddSuccessRepositories(repo scm.Repository) { } // AddSuccessPullRequest adds a pullrequest that succeeded -func (r *Counter) AddSuccessPullRequest(repo scm.PullRequest) { +func (r *Counter) AddSuccessPullRequest(pr scm.PullRequest) { defer r.lock.Unlock() r.lock.Lock() - r.successPullRequests = append(r.successPullRequests, repo) + r.successPullRequests = append(r.successPullRequests, pr) } // Info returns a formatted string about all repositories @@ -56,10 +64,18 @@ func (r *Counter) Info() string { var exitInfo string - for errMsg := range r.errorRepositories { + for errMsg := range r.errors { exitInfo += fmt.Sprintf("%s:\n", strings.ToUpper(errMsg[0:1])+errMsg[1:]) - for _, repo := range r.errorRepositories[errMsg] { - exitInfo += fmt.Sprintf(" %s\n", repo.FullName()) + for _, errInfo := range r.errors[errMsg] { + if errInfo.pullRequest == nil { + exitInfo += fmt.Sprintf(" %s\n", errInfo.repository.FullName()) + } else { + if urler, ok := errInfo.pullRequest.(urler); ok { + exitInfo += fmt.Sprintf(" %s\n", terminal.Link(errInfo.pullRequest.String(), urler.URL())) + } else { + exitInfo += fmt.Sprintf(" %s\n", errInfo.pullRequest.String()) + } + } } } diff --git a/internal/multigitter/run.go b/internal/multigitter/run.go index 072b6df5..0c452e45 100755 --- a/internal/multigitter/run.go +++ b/internal/multigitter/run.go @@ -121,16 +121,17 @@ func (r *Runner) Run(ctx context.Context) error { defer func() { if r := recover(); r != nil { log.Error(r) - rc.AddError(errors.New("run paniced"), repos[i]) + rc.AddError(errors.New("run paniced"), repos[i], nil) } }() pr, err := r.runSingleRepo(ctx, repos[i]) + if err != nil { if err != errAborted { logger.Info(err) } - rc.AddError(err, repos[i]) + rc.AddError(err, repos[i], pr) if log.IsLevelEnabled(log.TraceLevel) { if stackTrace := getStackTrace(err); stackTrace != "" { @@ -296,7 +297,12 @@ func (r *Runner) runSingleRepo(ctx context.Context, repo scm.Repository) (scm.Pu if err != nil { return nil, errors.Wrap(err, "could not verify if branch already exists") } else if featureBranchExist && r.ConflictStrategy == ConflictStrategySkip { - return nil, errBranchExist + pr, err := r.ensurePullRequestExists(ctx, log, repo, prRepo, baseBranch, featureBranchExist) + if err != nil { + return nil, err + } + + return pr, errBranchExist } } @@ -307,6 +313,10 @@ func (r *Runner) runSingleRepo(ctx context.Context, repo scm.Repository) (scm.Pu return nil, errors.Wrap(err, "could not push changes") } + return r.ensurePullRequestExists(ctx, log, repo, prRepo, baseBranch, featureBranchExist) +} + +func (r *Runner) ensurePullRequestExists(ctx context.Context, log log.FieldLogger, repo scm.Repository, prRepo scm.Repository, baseBranch string, featureBranchExist bool) (scm.PullRequest, error) { if r.SkipPullRequest { return nil, nil } @@ -321,29 +331,23 @@ func (r *Runner) runSingleRepo(ctx context.Context, repo scm.Repository) (scm.Pu existingPullRequest = pr } - var pr scm.PullRequest if existingPullRequest != nil { log.Info("Skip creating pull requests since one is already open") - pr = existingPullRequest - } else { - log.Info("Creating pull request") - pr, err = r.VersionController.CreatePullRequest(ctx, repo, prRepo, scm.NewPullRequest{ - Title: r.PullRequestTitle, - Body: r.PullRequestBody, - Head: r.FeatureBranch, - Base: baseBranch, - Reviewers: getReviewers(r.Reviewers, r.MaxReviewers), - TeamReviewers: getReviewers(r.TeamReviewers, r.MaxTeamReviewers), - Assignees: r.Assignees, - Draft: r.Draft, - Labels: r.Labels, - }) - if err != nil { - return nil, err - } + return existingPullRequest, nil } - return pr, nil + log.Info("Creating pull request") + return r.VersionController.CreatePullRequest(ctx, repo, prRepo, scm.NewPullRequest{ + Title: r.PullRequestTitle, + Body: r.PullRequestBody, + Head: r.FeatureBranch, + Base: baseBranch, + Reviewers: getReviewers(r.Reviewers, r.MaxReviewers), + TeamReviewers: getReviewers(r.TeamReviewers, r.MaxTeamReviewers), + Assignees: r.Assignees, + Draft: r.Draft, + Labels: r.Labels, + }) } var interactiveInfo = `(V)iew changes. (A)ccept or (R)eject` diff --git a/tests/table_test.go b/tests/table_test.go index 23e5b3ce..2323b301 100644 --- a/tests/table_test.go +++ b/tests/table_test.go @@ -378,11 +378,28 @@ func TestTable(t *testing.T) { changeBranch(t, repo.Path, "custom-branch-name", true) changeTestFile(t, repo.Path, "i like apple", "test change") changeBranch(t, repo.Path, "master", false) + + repoExistingPR := createRepo(t, "owner", "already-existing-branch-and-pr", "i like apples") + changeBranch(t, repoExistingPR.Path, "custom-branch-name", true) + changeTestFile(t, repoExistingPR.Path, "i like apple", "test change") + changeBranch(t, repoExistingPR.Path, "master", false) + return &vcmock.VersionController{ Repositories: []vcmock.Repository{ repo, + repoExistingPR, createRepo(t, "owner", "should-change", "i like apples"), }, + PullRequests: []vcmock.PullRequest{ + { + PRStatus: scm.PullRequestStatusPending, + PRNumber: 10, + Repository: repoExistingPR, + NewPullRequest: scm.NewPullRequest{ + Head: "custom-branch-name", + }, + }, + }, } }, args: []string{ @@ -394,14 +411,15 @@ func TestTable(t *testing.T) { changerBinaryPath, }, verify: func(t *testing.T, vcMock *vcmock.VersionController, runData runData) { - require.Len(t, vcMock.PullRequests, 1) - assert.Contains(t, runData.logOut, "Running on 2 repositories") + require.Len(t, vcMock.PullRequests, 3) + assert.Contains(t, runData.logOut, "Running on 3 repositories") assert.Contains(t, runData.logOut, "Cloning and running script") assert.Equal(t, `The new branch already exists: - owner/already-existing-branch + owner/already-existing-branch #1 + owner/already-existing-branch-and-pr #10 Repositories with a successful run: - owner/should-change #1 + owner/should-change #2 `, runData.out) }, },