Skip to content

Commit 8f3ac1e

Browse files
committed
use combine ops per @nobe4's suggestion
1 parent 34e089c commit 8f3ac1e

File tree

2 files changed

+37
-23
lines changed

2 files changed

+37
-23
lines changed

internal/cmd/combine_prs.go

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,56 +20,63 @@ type RESTClientInterface interface {
2020
Patch(endpoint string, body io.Reader, response interface{}) error
2121
}
2222

23+
// CombineOpts holds options for combining PRs
24+
// Use this struct to pass options to CombinePRsWithStats and related functions
25+
// This makes the code more maintainable and clear
26+
type CombineOpts struct {
27+
Noop bool
28+
Command string
29+
Repo github.Repo
30+
Pulls github.Pulls
31+
}
32+
2333
// CombinePRsWithStats combines PRs and returns stats for summary output
24-
func CombinePRsWithStats(ctx context.Context, graphQlClient *api.GraphQLClient, restClient RESTClientInterface, repo github.Repo, pulls github.Pulls, command string, dryRun bool) (combined []string, mergeConflicts []string, combinedPRLink string, err error) {
25-
// Move variable definitions outside the conditional blocks
34+
func CombinePRsWithStats(ctx context.Context, graphQlClient *api.GraphQLClient, restClient RESTClientInterface, opts CombineOpts) (combined []string, mergeConflicts []string, combinedPRLink string, err error) {
2635
workingBranchName := combineBranchName + workingBranchSuffix
2736

28-
repoDefaultBranch, err := getDefaultBranch(ctx, restClient, repo)
37+
repoDefaultBranch, err := getDefaultBranch(ctx, restClient, opts.Repo)
2938
if err != nil {
3039
return nil, nil, "", fmt.Errorf("failed to get default branch: %w", err)
3140
}
3241

33-
baseBranchSHA, err := getBranchSHA(ctx, restClient, repo, repoDefaultBranch)
42+
baseBranchSHA, err := getBranchSHA(ctx, restClient, opts.Repo, repoDefaultBranch)
3443
if err != nil {
3544
return nil, nil, "", fmt.Errorf("failed to get SHA of main branch: %w", err)
3645
}
3746

38-
if dryRun {
47+
if opts.Noop {
3948
Logger.Debug("Dry-run mode enabled. No changes will be made.")
4049
Logger.Debug("Simulating branch operations", "workingBranch", workingBranchName, "defaultBranch", repoDefaultBranch)
4150
}
4251

43-
// Skip branch creation and deletion if dry-run is enabled
44-
if !dryRun {
45-
err = deleteBranch(ctx, restClient, repo, workingBranchName)
52+
if !opts.Noop {
53+
err = deleteBranch(ctx, restClient, opts.Repo, workingBranchName)
4654
if err != nil {
4755
Logger.Debug("Working branch not found, continuing", "branch", workingBranchName)
4856
}
4957

50-
err = deleteBranch(ctx, restClient, repo, combineBranchName)
58+
err = deleteBranch(ctx, restClient, opts.Repo, combineBranchName)
5159
if err != nil {
5260
Logger.Debug("Combined branch not found, continuing", "branch", combineBranchName)
5361
}
5462

55-
err = createBranch(ctx, restClient, repo, combineBranchName, baseBranchSHA)
63+
err = createBranch(ctx, restClient, opts.Repo, combineBranchName, baseBranchSHA)
5664
if err != nil {
5765
return nil, nil, "", fmt.Errorf("failed to create combined branch: %w", err)
5866
}
5967

60-
err = createBranch(ctx, restClient, repo, workingBranchName, baseBranchSHA)
68+
err = createBranch(ctx, restClient, opts.Repo, workingBranchName, baseBranchSHA)
6169
if err != nil {
6270
return nil, nil, "", fmt.Errorf("failed to create working branch: %w", err)
6371
}
6472
}
6573

66-
// Simulate merging PRs
67-
for _, pr := range pulls {
68-
if dryRun {
74+
for _, pr := range opts.Pulls {
75+
if opts.Noop {
6976
Logger.Debug("Simulating merge of branch", "branch", pr.Head.Ref)
7077
combined = append(combined, fmt.Sprintf("#%d - %s", pr.Number, pr.Title))
7178
} else {
72-
err := mergeBranch(ctx, restClient, repo, workingBranchName, pr.Head.Ref)
79+
err := mergeBranch(ctx, restClient, opts.Repo, workingBranchName, pr.Head.Ref)
7380
if err != nil {
7481
if isMergeConflictError(err) {
7582
Logger.Debug("Merge conflict", "branch", pr.Head.Ref, "error", err)
@@ -84,25 +91,25 @@ func CombinePRsWithStats(ctx context.Context, graphQlClient *api.GraphQLClient,
8491
}
8592
}
8693

87-
if !dryRun {
88-
err = updateRef(ctx, restClient, repo, combineBranchName, workingBranchName)
94+
if !opts.Noop {
95+
err = updateRef(ctx, restClient, opts.Repo, combineBranchName, workingBranchName)
8996
if err != nil {
9097
return combined, mergeConflicts, "", fmt.Errorf("failed to update combined branch: %w", err)
9198
}
9299

93-
err = deleteBranch(ctx, restClient, repo, workingBranchName)
100+
err = deleteBranch(ctx, restClient, opts.Repo, workingBranchName)
94101
if err != nil {
95102
Logger.Warn("Failed to delete working branch", "branch", workingBranchName, "error", err)
96103
}
97104

98-
prBody := generatePRBody(combined, mergeConflicts, command)
105+
prBody := generatePRBody(combined, mergeConflicts, opts.Command)
99106
prTitle := "Combined PRs"
100-
prNumber, prErr := createPullRequestWithNumber(ctx, restClient, repo, prTitle, combineBranchName, repoDefaultBranch, prBody, addLabels, addAssignees)
107+
prNumber, prErr := createPullRequestWithNumber(ctx, restClient, opts.Repo, prTitle, combineBranchName, repoDefaultBranch, prBody, addLabels, addAssignees)
101108
if prErr != nil {
102109
return combined, mergeConflicts, "", fmt.Errorf("failed to create combined PR: %w", prErr)
103110
}
104111
if prNumber > 0 {
105-
combinedPRLink = fmt.Sprintf("https://github.com/%s/%s/pull/%d", repo.Owner, repo.Repo, prNumber)
112+
combinedPRLink = fmt.Sprintf("https://github.com/%s/%s/pull/%d", opts.Repo.Owner, opts.Repo.Repo, prNumber)
106113
}
107114
}
108115

internal/cmd/root.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,16 @@ func processRepository(ctx context.Context, client *api.RESTClient, graphQlClien
339339
RESTClientInterface
340340
}{client}
341341

342-
// Combine the PRs and collect stats
343342
commandString := buildCommandString([]string{repo.String()})
344-
combined, mergeConflicts, combinedPRLink, err := CombinePRsWithStats(ctx, graphQlClient, restClientWrapper, repo, matchedPRs, commandString, dryRun)
343+
344+
opts := CombineOpts{
345+
Noop: dryRun,
346+
Command: commandString,
347+
Repo: repo,
348+
Pulls: matchedPRs,
349+
}
350+
351+
combined, mergeConflicts, combinedPRLink, err := CombinePRsWithStats(ctx, graphQlClient, restClientWrapper, opts)
345352
if err != nil {
346353
return fmt.Errorf("failed to combine PRs: %w", err)
347354
}

0 commit comments

Comments
 (0)