Skip to content

Commit

Permalink
fix: added the number of created pull requests (#56)
Browse files Browse the repository at this point in the history
  • Loading branch information
lindell authored Jan 19, 2021
1 parent 06c90aa commit d432430
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 48 deletions.
13 changes: 9 additions & 4 deletions internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,24 @@ func (g Github) getRepository(ctx context.Context, repoRef RepositoryReference)
}

// CreatePullRequest creates a pull request
func (g Github) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error {
func (g Github) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) {
r := repo.(repository)

pr, err := g.createPullRequest(ctx, r, newPR)
if err != nil {
return err
return nil, err
}

if err := g.addReviewers(ctx, r, newPR, pr); err != nil {
return err
return nil, err
}

return nil
return pullRequest{
ownerName: pr.GetBase().GetUser().GetLogin(),
repoName: pr.GetBase().GetRepo().GetName(),
branchName: pr.GetHead().GetRef(),
number: pr.GetNumber(),
}, nil
}

func (g Github) createPullRequest(ctx context.Context, repo repository, newPR domain.NewPullRequest) (*github.PullRequest, error) {
Expand Down
18 changes: 14 additions & 4 deletions internal/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (g *Gitlab) getUserProjects(ctx context.Context, username string) ([]*gitla
}

// CreatePullRequest creates a pull request
func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error {
func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) {
r := repo.(repository)

// Convert from usernames to user ids
Expand All @@ -241,18 +241,28 @@ func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository,
var err error
assigneeIDs, err = g.getUserIDs(ctx, newPR.Reviewers)
if err != nil {
return err
return nil, err
}
}

_, _, err := g.glClient.MergeRequests.CreateMergeRequest(r.pid, &gitlab.CreateMergeRequestOptions{
mr, _, err := g.glClient.MergeRequests.CreateMergeRequest(r.pid, &gitlab.CreateMergeRequestOptions{
Title: &newPR.Title,
Description: &newPR.Body,
SourceBranch: &newPR.Head,
TargetBranch: &newPR.Base,
AssigneeIDs: assigneeIDs,
})
return err
if err != nil {
return nil, err
}

return pullRequest{
repoName: r.name,
ownerName: r.ownerName,
pid: r.pid,
branchName: newPR.Head,
iid: mr.IID,
}, nil
}

func (g *Gitlab) getUserIDs(ctx context.Context, usernames []string) ([]int, error) {
Expand Down
2 changes: 1 addition & 1 deletion internal/multigitter/close.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (s Closer) Close(ctx context.Context) error {
log.Infof("Closing %d pull requests", len(openPRs))

for _, pr := range openPRs {
log.WithField("repo", pr.String()).Infof("Closing")
log.WithField("pr", pr.String()).Infof("Closing")
err := s.VersionController.ClosePullRequest(ctx, pr)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion internal/multigitter/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (s Merger) Merge(ctx context.Context) error {
log.Infof("Merging %d pull requests", len(successPrs))

for _, pr := range successPrs {
log.WithField("repo", pr.String()).Infof("Merging")
log.WithField("pr", pr.String()).Infof("Merging")
err := s.VersionController.MergePullRequest(ctx, pr)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion internal/multigitter/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (r Printer) Print(ctx context.Context) error {
return
}

rc.AddSuccess(repos[i])
rc.AddSuccessRepositories(repos[i])
}, len(repos), r.Concurrent)

return nil
Expand Down
18 changes: 17 additions & 1 deletion internal/multigitter/repocounter/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

// Counter keeps track of succeeded and failed repositories
type Counter struct {
successPullRequests []domain.PullRequest
successRepositories []domain.Repository
errorRepositories map[string][]domain.Repository
lock sync.RWMutex
Expand All @@ -32,13 +33,21 @@ func (r *Counter) AddError(err error, repo domain.Repository) {
}

// AddSuccess adds a repository that succeeded
func (r *Counter) AddSuccess(repo domain.Repository) {
func (r *Counter) AddSuccessRepositories(repo domain.Repository) {
defer r.lock.Unlock()
r.lock.Lock()

r.successRepositories = append(r.successRepositories, repo)
}

// AddSuccess adds a pullrequest that succeeded
func (r *Counter) AddSuccessPullRequest(repo domain.PullRequest) {
defer r.lock.Unlock()
r.lock.Lock()

r.successPullRequests = append(r.successPullRequests, repo)
}

// Info returns a formated string about all repositories
func (r *Counter) Info() string {
defer r.lock.RUnlock()
Expand All @@ -53,6 +62,13 @@ func (r *Counter) Info() string {
}
}

if len(r.successPullRequests) > 0 {
exitInfo += "Repositories with a successful run:\n"
for _, pr := range r.successPullRequests {
exitInfo += fmt.Sprintf(" %s\n", pr.String())
}
}

if len(r.successRepositories) > 0 {
exitInfo += "Repositories with a successful run:\n"
for _, repo := range r.successRepositories {
Expand Down
53 changes: 34 additions & 19 deletions internal/multigitter/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
// VersionController fetches repositories
type VersionController interface {
GetRepositories(ctx context.Context) ([]domain.Repository, error)
CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error
CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error)
GetPullRequestStatuses(ctx context.Context, branchName string) ([]domain.PullRequest, error)
MergePullRequest(ctx context.Context, pr domain.PullRequest) error
ClosePullRequest(ctx context.Context, pr domain.PullRequest) error
Expand Down Expand Up @@ -53,6 +53,19 @@ type Runner struct {

var errAborted = errors.New("run was never started because of aborted execution")

type dryRunPullRequest struct {
status domain.PullRequestStatus
Repository domain.Repository
}

func (pr dryRunPullRequest) Status() domain.PullRequestStatus {
return pr.status
}

func (pr dryRunPullRequest) String() string {
return fmt.Sprintf("%s #0", pr.Repository.FullName())
}

// Run runs a script for multiple repositories and creates PRs with the changes made
func (r Runner) Run(ctx context.Context) error {
repos, err := r.VersionController.GetRepositories(ctx)
Expand All @@ -71,7 +84,7 @@ func (r Runner) Run(ctx context.Context) error {

runInParallel(func(i int) {
logger := log.WithField("repo", repos[i].FullName())
err := r.runSingleRepo(ctx, repos[i])
pr, err := r.runSingleRepo(ctx, repos[i])
if err != nil {
if err != errAborted {
logger.Info(err)
Expand All @@ -80,7 +93,7 @@ func (r Runner) Run(ctx context.Context) error {
return
}

rc.AddSuccess(repos[i])
rc.AddSuccessPullRequest(pr)
}, len(repos), r.Concurrent)

return nil
Expand Down Expand Up @@ -111,17 +124,17 @@ func getReviewers(reviewers []string, maxReviewers int) []string {
return reviewers[0:maxReviewers]
}

func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error {
func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) (domain.PullRequest, error) {
if ctx.Err() != nil {
return errAborted
return nil, errAborted
}

log := log.WithField("repo", repo.FullName())
log.Info("Cloning and running script")

tmpDir, err := ioutil.TempDir(os.TempDir(), "multi-git-changer-")
if err != nil {
return err
return nil, err
}
defer os.RemoveAll(tmpDir)

Expand All @@ -137,19 +150,19 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error

err = sourceController.Clone(baseBranch, r.FeatureBranch)
if err != nil {
return err
return nil, err
}

featureBranchExist, err := sourceController.BranchExist(r.FeatureBranch)
if err != nil {
return errors.Wrap(err, "could not verify if branch already exist")
return nil, errors.Wrap(err, "could not verify if branch already exist")
} else if featureBranchExist {
return domain.BranchExistError
return nil, domain.BranchExistError
}

err = sourceController.ChangeBranch(r.FeatureBranch)
if err != nil {
return err
return nil, err
}

// Run the command that might or might not change the content of the repo
Expand All @@ -167,41 +180,43 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error

err = cmd.Run()
if err != nil {
return err
return nil, err
}

if changed, err := sourceController.Changes(); err != nil {
return err
return nil, err
} else if !changed {
return domain.NoChangeError
return nil, domain.NoChangeError
}

err = sourceController.Commit(r.CommitAuthor, r.CommitMessage)
if err != nil {
return err
return nil, err
}

if r.DryRun {
log.Info("Skipping pushing changes because of dry run")
return nil
return dryRunPullRequest{
Repository: repo,
}, nil
}

err = sourceController.Push()
if err != nil {
return errors.Wrap(err, "could not push changes")
return nil, errors.Wrap(err, "could not push changes")
}

log.Info("Change done, creating pull request")
err = r.VersionController.CreatePullRequest(ctx, repo, domain.NewPullRequest{
pr, err := r.VersionController.CreatePullRequest(ctx, repo, domain.NewPullRequest{
Title: r.PullRequestTitle,
Body: r.PullRequestBody,
Head: r.FeatureBranch,
Base: baseBranch,
Reviewers: getReviewers(r.Reviewers, r.MaxReviewers),
})
if err != nil {
return err
return nil, err
}

return nil
return pr, nil
}
18 changes: 9 additions & 9 deletions tests/story_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ func TestStory(t *testing.T) {
// Verify that the output was correct
runOutData, err := ioutil.ReadFile(runOutFile)
require.NoError(t, err)
assert.Equal(t, string(runOutData), `No data was changed:
assert.Equal(t, `No data was changed:
should-not-change
Repositories with a successful run:
should-change
should-change-2
`)
should-change #1
should-change-2 #2
`, string(runOutData))

//
// Status
Expand All @@ -81,7 +81,7 @@ Repositories with a successful run:
// Verify that the output was correct
statusOutData, err := ioutil.ReadFile(statusOutFile)
require.NoError(t, err)
assert.Equal(t, "should-change/XX: Pending\nshould-change-2/XX: Pending\n", string(statusOutData))
assert.Equal(t, "should-change #1: Pending\nshould-change-2 #2: Pending\n", string(statusOutData))

// One of the created PRs is set to succeeded
vcMock.SetPRStatus("should-change", "custom-branch-name", domain.PullRequestStatusSuccess)
Expand All @@ -103,7 +103,7 @@ Repositories with a successful run:
mergeLogData, err := ioutil.ReadFile(mergeLogFile)
require.NoError(t, err)
assert.Contains(t, string(mergeLogData), "Merging 1 pull requests")
assert.Contains(t, string(mergeLogData), "Merging repo=should-change/XX")
assert.Contains(t, string(mergeLogData), "Merging pr=\"should-change #1\"")

//
// After Merge Status
Expand All @@ -121,7 +121,7 @@ Repositories with a successful run:
// Verify that the output was correct
afterMergeStatusOutData, err := ioutil.ReadFile(afterMergeStatusOutFile)
require.NoError(t, err)
assert.Equal(t, "should-change/XX: Merged\nshould-change-2/XX: Pending\n", string(afterMergeStatusOutData))
assert.Equal(t, "should-change #1: Merged\nshould-change-2 #2: Pending\n", string(afterMergeStatusOutData))

//
// Close
Expand All @@ -140,7 +140,7 @@ Repositories with a successful run:
closeLogData, err := ioutil.ReadFile(closeLogFile)
require.NoError(t, err)
assert.Contains(t, string(closeLogData), "Closing 1 pull request")
assert.Contains(t, string(closeLogData), "Closing repo=should-change-2/XX")
assert.Contains(t, string(closeLogData), "Closing pr=\"should-change-2 #2\"")

//
// After Close Status
Expand All @@ -158,5 +158,5 @@ Repositories with a successful run:
// Verify that the output was correct
afterCloseStatusOutData, err := ioutil.ReadFile(afterCloseStatusOutFile)
require.NoError(t, err)
assert.Equal(t, "should-change/XX: Merged\nshould-change-2/XX: Closed\n", string(afterCloseStatusOutData))
assert.Equal(t, "should-change #1: Merged\nshould-change-2 #2: Closed\n", string(afterCloseStatusOutData))
}
6 changes: 3 additions & 3 deletions tests/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestTable(t *testing.T) {
assert.Contains(t, runData.logOut, "Change done, creating pull request")

assert.Equal(t, `Repositories with a successful run:
should-change
should-change #1
`, runData.out)
},
},
Expand Down Expand Up @@ -90,7 +90,7 @@ func TestTable(t *testing.T) {
assert.Contains(t, runData.logOut, "Change done, creating pull request")

assert.Equal(t, `Repositories with a successful run:
should-change
should-change #1
`, runData.out)
},
},
Expand Down Expand Up @@ -275,7 +275,7 @@ func TestTable(t *testing.T) {
assert.Equal(t, `The new branch does already exist:
already-existing-branch
Repositories with a successful run:
should-change
should-change #1
`, runData.out)
},
},
Expand Down
Loading

0 comments on commit d432430

Please sign in to comment.