From b14e15d9abe3e10dd22ada83e1970e4789c7b2d2 Mon Sep 17 00:00:00 2001
From: nobe4 <nobe4@users.noreply.github.com>
Date: Tue, 8 Apr 2025 09:20:40 +0200
Subject: [PATCH 1/8] docs(root): indent the help properly
---
internal/cmd/root.go | 94 ++++++++++++++++++++++----------------------
1 file changed, 47 insertions(+), 47 deletions(-)
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
index 1fbe244..538a1b1 100644
--- a/internal/cmd/root.go
+++ b/internal/cmd/root.go
@@ -41,53 +41,53 @@ func NewRootCmd() *cobra.Command {
Use: "combine owner/repo",
Short: "Combine multiple pull requests into a single PR",
Long: `Combine multiple pull requests that match specific criteria into a single PR.
- Examples:
- # Basic usage with a single repository (will default to "--branch-prefix dependabot/" and "--minimum 2")
- gh combine octocat/hello-world
-
- # Multiple repositories (comma-separated)
- gh combine octocat/repo1,octocat/repo2
-
- # Multiple repositories (no commas)
- gh combine octocat/repo1 octocat/repo2
-
- # Using default owner for repositories
- gh combine --owner octocat repo1 repo2
-
- # Using default owner for only some repositories
- gh combine --owner octocat repo1 octocat/repo2
-
- # Using a file with repository names (one per line: owner/repo format)
- gh combine --file repos.txt
-
- # Filter PRs by branch name
- gh combine octocat/hello-world --branch-prefix dependabot/ # Only include PRs with the standard dependabot branch prefix
- gh combine octocat/hello-world --branch-suffix -update
- gh combine octocat/hello-world --branch-regex "dependabot/.*"
-
- # Filter PRs by labels
- gh combine octocat/hello-world --label dependencies # PRs must have this single label
- gh combine octocat/hello-world --labels security,dependencies # PRs must have ALL these labels
-
- # Exclude PRs by labels
- gh combine octocat/hello-world --ignore-label wip # Ignore PRs with this label
- gh combine octocat/hello-world --ignore-labels wip,draft # Ignore PRs with ANY of these labels
-
- # Set requirements for PRs to be combined
- gh combine octocat/hello-world --require-ci # Only include PRs with passing CI
- gh combine octocat/hello-world --require-approved # Only include approved PRs
- gh combine octocat/hello-world --minimum 3 # Need at least 3 matching PRs
-
- # Add metadata to combined PR
- gh combine octocat/hello-world --add-labels security,dependencies # Add these labels to the new PR
- gh combine octocat/hello-world --add-assignees octocat,hubot # Assign users to the new PR
-
- # Additional options
- gh combine octocat/hello-world --autoclose # Close source PRs when combined PR is merged
- gh combine octocat/hello-world --base-branch main # Use a different base branch for the combined PR
- gh combine octocat/hello-world --combine-branch-name combined-prs # Use a different name for the combined PR branch
- gh combine octocat/hello-world --working-branch-suffix -working # Use a different suffix for the working branch
- gh combine octocat/hello-world --update-branch # Update the branch of the combined PR`,
+ Examples:
+ # Basic usage with a single repository (will default to "--branch-prefix dependabot/" and "--minimum 2")
+ gh combine octocat/hello-world
+
+ # Multiple repositories (comma-separated)
+ gh combine octocat/repo1,octocat/repo2
+
+ # Multiple repositories (no commas)
+ gh combine octocat/repo1 octocat/repo2
+
+ # Using default owner for repositories
+ gh combine --owner octocat repo1 repo2
+
+ # Using default owner for only some repositories
+ gh combine --owner octocat repo1 octocat/repo2
+
+ # Using a file with repository names (one per line: owner/repo format)
+ gh combine --file repos.txt
+
+ # Filter PRs by branch name
+ gh combine octocat/hello-world --branch-prefix dependabot/ # Only include PRs with the standard dependabot branch prefix
+ gh combine octocat/hello-world --branch-suffix -update
+ gh combine octocat/hello-world --branch-regex "dependabot/.*"
+
+ # Filter PRs by labels
+ gh combine octocat/hello-world --label dependencies # PRs must have this single label
+ gh combine octocat/hello-world --labels security,dependencies # PRs must have ALL these labels
+
+ # Exclude PRs by labels
+ gh combine octocat/hello-world --ignore-label wip # Ignore PRs with this label
+ gh combine octocat/hello-world --ignore-labels wip,draft # Ignore PRs with ANY of these labels
+
+ # Set requirements for PRs to be combined
+ gh combine octocat/hello-world --require-ci # Only include PRs with passing CI
+ gh combine octocat/hello-world --require-approved # Only include approved PRs
+ gh combine octocat/hello-world --minimum 3 # Need at least 3 matching PRs
+
+ # Add metadata to combined PR
+ gh combine octocat/hello-world --add-labels security,dependencies # Add these labels to the new PR
+ gh combine octocat/hello-world --add-assignees octocat,hubot # Assign users to the new PR
+
+ # Additional options
+ gh combine octocat/hello-world --autoclose # Close source PRs when combined PR is merged
+ gh combine octocat/hello-world --base-branch main # Use a different base branch for the combined PR
+ gh combine octocat/hello-world --combine-branch-name combined-prs # Use a different name for the combined PR branch
+ gh combine octocat/hello-world --working-branch-suffix -working # Use a different suffix for the working branch
+ gh combine octocat/hello-world --update-branch # Update the branch of the combined PR`,
RunE: runCombine,
}
From 3095bc2a0f4a4cd522cb45423ac38d8188ae14eb Mon Sep 17 00:00:00 2001
From: nobe4 <nobe4@users.noreply.github.com>
Date: Tue, 8 Apr 2025 09:24:16 +0200
Subject: [PATCH 2/8] fix(root): use the example field that's dedicated
---
internal/cmd/root.go | 96 ++++++++++++++++++++++----------------------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
index 538a1b1..ffa9e46 100644
--- a/internal/cmd/root.go
+++ b/internal/cmd/root.go
@@ -40,54 +40,54 @@ func NewRootCmd() *cobra.Command {
rootCmd := &cobra.Command{
Use: "combine owner/repo",
Short: "Combine multiple pull requests into a single PR",
- Long: `Combine multiple pull requests that match specific criteria into a single PR.
- Examples:
- # Basic usage with a single repository (will default to "--branch-prefix dependabot/" and "--minimum 2")
- gh combine octocat/hello-world
-
- # Multiple repositories (comma-separated)
- gh combine octocat/repo1,octocat/repo2
-
- # Multiple repositories (no commas)
- gh combine octocat/repo1 octocat/repo2
-
- # Using default owner for repositories
- gh combine --owner octocat repo1 repo2
-
- # Using default owner for only some repositories
- gh combine --owner octocat repo1 octocat/repo2
-
- # Using a file with repository names (one per line: owner/repo format)
- gh combine --file repos.txt
-
- # Filter PRs by branch name
- gh combine octocat/hello-world --branch-prefix dependabot/ # Only include PRs with the standard dependabot branch prefix
- gh combine octocat/hello-world --branch-suffix -update
- gh combine octocat/hello-world --branch-regex "dependabot/.*"
-
- # Filter PRs by labels
- gh combine octocat/hello-world --label dependencies # PRs must have this single label
- gh combine octocat/hello-world --labels security,dependencies # PRs must have ALL these labels
-
- # Exclude PRs by labels
- gh combine octocat/hello-world --ignore-label wip # Ignore PRs with this label
- gh combine octocat/hello-world --ignore-labels wip,draft # Ignore PRs with ANY of these labels
-
- # Set requirements for PRs to be combined
- gh combine octocat/hello-world --require-ci # Only include PRs with passing CI
- gh combine octocat/hello-world --require-approved # Only include approved PRs
- gh combine octocat/hello-world --minimum 3 # Need at least 3 matching PRs
-
- # Add metadata to combined PR
- gh combine octocat/hello-world --add-labels security,dependencies # Add these labels to the new PR
- gh combine octocat/hello-world --add-assignees octocat,hubot # Assign users to the new PR
-
- # Additional options
- gh combine octocat/hello-world --autoclose # Close source PRs when combined PR is merged
- gh combine octocat/hello-world --base-branch main # Use a different base branch for the combined PR
- gh combine octocat/hello-world --combine-branch-name combined-prs # Use a different name for the combined PR branch
- gh combine octocat/hello-world --working-branch-suffix -working # Use a different suffix for the working branch
- gh combine octocat/hello-world --update-branch # Update the branch of the combined PR`,
+ Long: `Combine multiple pull requests that match specific criteria into a single PR.`,
+ Example: `
+ # Basic usage with a single repository (will default to "--branch-prefix dependabot/" and "--minimum 2")
+ gh combine octocat/hello-world
+
+ # Multiple repositories (comma-separated)
+ gh combine octocat/repo1,octocat/repo2
+
+ # Multiple repositories (no commas)
+ gh combine octocat/repo1 octocat/repo2
+
+ # Using default owner for repositories
+ gh combine --owner octocat repo1 repo2
+
+ # Using default owner for only some repositories
+ gh combine --owner octocat repo1 octocat/repo2
+
+ # Using a file with repository names (one per line: owner/repo format)
+ gh combine --file repos.txt
+
+ # Filter PRs by branch name
+ gh combine octocat/hello-world --branch-prefix dependabot/ # Only include PRs with the standard dependabot branch prefix
+ gh combine octocat/hello-world --branch-suffix -update
+ gh combine octocat/hello-world --branch-regex "dependabot/.*"
+
+ # Filter PRs by labels
+ gh combine octocat/hello-world --label dependencies # PRs must have this single label
+ gh combine octocat/hello-world --labels security,dependencies # PRs must have ALL these labels
+
+ # Exclude PRs by labels
+ gh combine octocat/hello-world --ignore-label wip # Ignore PRs with this label
+ gh combine octocat/hello-world --ignore-labels wip,draft # Ignore PRs with ANY of these labels
+
+ # Set requirements for PRs to be combined
+ gh combine octocat/hello-world --require-ci # Only include PRs with passing CI
+ gh combine octocat/hello-world --require-approved # Only include approved PRs
+ gh combine octocat/hello-world --minimum 3 # Need at least 3 matching PRs
+
+ # Add metadata to combined PR
+ gh combine octocat/hello-world --add-labels security,dependencies # Add these labels to the new PR
+ gh combine octocat/hello-world --add-assignees octocat,hubot # Assign users to the new PR
+
+ # Additional options
+ gh combine octocat/hello-world --autoclose # Close source PRs when combined PR is merged
+ gh combine octocat/hello-world --base-branch main # Use a different base branch for the combined PR
+ gh combine octocat/hello-world --combine-branch-name combined-prs # Use a different name for the combined PR branch
+ gh combine octocat/hello-world --working-branch-suffix -working # Use a different suffix for the working branch
+ gh combine octocat/hello-world --update-branch # Update the branch of the combined PR`,
RunE: runCombine,
}
From c3a6495d26f893c07ff971fc538b4e7f4270d1f5 Mon Sep 17 00:00:00 2001
From: nobe4 <nobe4@users.noreply.github.com>
Date: Tue, 8 Apr 2025 09:45:05 +0200
Subject: [PATCH 3/8] feat(input): add better error handling and tests
---
internal/cmd/inputs.go | 77 ++++++++++++++++++++++---------------
internal/cmd/inputs_test.go | 63 ++++++++++++++++++++++++++++++
2 files changed, 108 insertions(+), 32 deletions(-)
diff --git a/internal/cmd/inputs.go b/internal/cmd/inputs.go
index 05236f3..f46a4c1 100644
--- a/internal/cmd/inputs.go
+++ b/internal/cmd/inputs.go
@@ -3,55 +3,68 @@ package cmd
import (
"errors"
"fmt"
+ "slices"
+)
+
+// @grant: whenever possible, use sentinel errors. This ensures that those
+// values can be testsed with `errors.Is()` correctly.
+var (
+ errNoRepo = errors.New("must specify repositories or provide a file containing a list of repositories with --file")
+ errLabelConflict = errors.New("--ignore-label(s) and --label(s) cannot have the same value")
+ errLabelsConflict = errors.New("--label(s) contains a value which conflicts with --ignore-label(s)")
+ errIgnoreLabelsConflict = errors.New("--ignore-label(s) contains a value which conflicts with --label(s)")
)
// validateInputs checks if the provided inputs are valid
func ValidateInputs(args []string) error {
+ // @grant: using global variables is terrible for tests, so here I've
+ // injected them into the function for validation.
+ if err := ValidateLabels(selectLabel, selectLabels, ignoreLabel, ignoreLabels); err != nil {
+ return err
+ }
+
+ // If no args and no file, we can't proceed
+ if len(args) == 0 && reposFile == "" {
+ return errNoRepo
+ }
+
+ // Warn if no filtering options are provided at all
+ if branchPrefix == "" && branchSuffix == "" && branchRegex == "" &&
+ ignoreLabel == "" && len(ignoreLabels) == 0 && selectLabel == "" && len(selectLabels) == 0 &&
+ !requireCI && !mustBeApproved {
+ Logger.Warn("No filtering options specified. This will attempt to combine ALL open pull requests.")
+ }
+
+ return nil
+}
+
+// @grant: this function is 100% self contained, and does the same logic as
+// before, which makes it really easier to test.
+func ValidateLabels(selectLabel string, selectLabels []string, ignoreLabel string, ignoreLabels []string) error {
// Check that ignore-label and select-label are not the same
if ignoreLabel != "" && selectLabel != "" && ignoreLabel == selectLabel {
- return errors.New("--ignore-label(s) and --label(s) cannot have the same value")
+ return errLabelConflict
}
+ // @grant: slices.Index beats a `for` and an `if`.
// Check for conflicts between ignoreLabels and selectLabel
- if selectLabel != "" && len(ignoreLabels) > 0 {
- for _, ignoreL := range ignoreLabels {
- if ignoreL == selectLabel {
- return fmt.Errorf("--ignore-labels contains %q which conflicts with --label %q", ignoreL, selectLabel)
- }
- }
+ if i := slices.Index(ignoreLabels, selectLabel); i != -1 {
+ return fmt.Errorf("%w: %q %q", errIgnoreLabelsConflict, ignoreLabels[i], selectLabel)
}
// Check for conflicts between ignoreLabel and selectLabels
- if ignoreLabel != "" && len(selectLabels) > 0 {
- for _, selectL := range selectLabels {
- if selectL == ignoreLabel {
- return fmt.Errorf("--label(s) contains %q which conflicts with --ignore-label %q", selectL, ignoreLabel)
- }
- }
+ if i := slices.Index(selectLabels, ignoreLabel); i != -1 {
+ return fmt.Errorf("%w: %q %q", errLabelsConflict, selectLabels[i], ignoreLabel)
}
+ // @grant: no need to check for the length if we iterate directly, the
+ // performance gain is moot.
// Check for conflicts between ignoreLabels and selectLabels
- if len(ignoreLabels) > 0 && len(selectLabels) > 0 {
- for _, ignoreL := range ignoreLabels {
- for _, selectL := range selectLabels {
- if ignoreL == selectL {
- return fmt.Errorf("--ignore-labels contains %q which conflicts with --labels containing the same value", ignoreL)
- }
- }
+ for _, ignoreL := range ignoreLabels {
+ if i := slices.Index(selectLabels, ignoreL); i != -1 {
+ return fmt.Errorf("%w: %q %q", errIgnoreLabelsConflict, ignoreLabels[i], selectLabel)
}
}
- // If no args and no file, we can't proceed
- if len(args) == 0 && reposFile == "" {
- return errors.New("must specify repositories or provide a file containing a list of repositories with --file")
- }
-
- // Warn if no filtering options are provided at all
- if branchPrefix == "" && branchSuffix == "" && branchRegex == "" &&
- ignoreLabel == "" && len(ignoreLabels) == 0 && selectLabel == "" && len(selectLabels) == 0 &&
- !requireCI && !mustBeApproved {
- Logger.Warn("No filtering options specified. This will attempt to combine ALL open pull requests.")
- }
-
return nil
}
diff --git a/internal/cmd/inputs_test.go b/internal/cmd/inputs_test.go
index 6d5528b..9c4bd29 100644
--- a/internal/cmd/inputs_test.go
+++ b/internal/cmd/inputs_test.go
@@ -2,6 +2,7 @@ package cmd
import (
"bytes"
+ "errors"
"log/slog"
"os"
"strings"
@@ -19,6 +20,68 @@ func setupMockLogger() (*bytes.Buffer, func()) {
}
}
+// @grant: this is a better pattern to write tests.
+// In a nutshell, you make each function as self-contained as possible, create
+// and iterate over a list of test cases and ensure the expected output is
+// received.
+func TestValidateLabels(t *testing.T) {
+ tests := []struct {
+ selectLabel string
+ selectLabels []string
+ ignoreLabel string
+ ignoreLabels []string
+ want error
+ }{
+ {
+ // empty labels
+ want: nil,
+ },
+
+ {
+ selectLabel: "enhancement",
+ ignoreLabel: "bug",
+ want: nil,
+ },
+
+ {
+ selectLabel: "enhancement",
+ ignoreLabel: "enhancement",
+ want: errLabelConflict,
+ },
+
+ {
+ selectLabel: "enhancement",
+ ignoreLabels: []string{"bug", "enhancement"},
+ want: errIgnoreLabelsConflict,
+ },
+
+ {
+ selectLabels: []string{"bug", "enhancement"},
+ ignoreLabel: "enhancement",
+ want: errLabelsConflict,
+ },
+
+ {
+ selectLabels: []string{"bug", "enhancement"},
+ ignoreLabels: []string{"request", "bug"},
+ want: errIgnoreLabelsConflict,
+ },
+
+ {
+ selectLabels: []string{"bug", "enhancement"},
+ ignoreLabels: []string{"request", "docs"},
+ want: nil,
+ },
+ }
+
+ for _, test := range tests {
+ got := ValidateLabels(test.selectLabel, test.selectLabels, test.ignoreLabel, test.ignoreLabels)
+ if !errors.Is(got, test.want) {
+ t.Fatalf("want %v, got %v", test.want, got)
+ }
+ }
+}
+
func TestValidateInputs_NoReposSpecified(t *testing.T) {
// Save original values
origReposFile := reposFile
From 519b299ff3d89cc813da89f449c57edfc9a65ab5 Mon Sep 17 00:00:00 2001
From: nobe4 <nobe4@users.noreply.github.com>
Date: Tue, 8 Apr 2025 09:48:48 +0200
Subject: [PATCH 4/8] fix(root): small improvements
- handle error correctly
- remove unused parameter
---
internal/cmd/root.go | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
index ffa9e46..246b31d 100644
--- a/internal/cmd/root.go
+++ b/internal/cmd/root.go
@@ -172,9 +172,13 @@ func runCombine(cmd *cobra.Command, args []string) error {
func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string) error {
// Create GitHub API client
restClient, err := api.DefaultRESTClient()
+ if err != nil {
+ return fmt.Errorf("failed to get the default REST client: %w", err)
+ }
+
graphQlClient, err := api.DefaultGraphQLClient()
if err != nil {
- return fmt.Errorf("failed to create REST client: %w", err)
+ return fmt.Errorf("failed to create the default GraphQL Client : %w", err)
}
for _, repo := range repos {
@@ -190,7 +194,7 @@ func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string
Logger.Debug("Processing repository", "repo", repo)
// Process the repository
- if err := processRepository(ctx, restClient, graphQlClient, spinner, repo); err != nil {
+ if err := processRepository(ctx, restClient, graphQlClient, repo); err != nil {
if ctx.Err() != nil {
// If the context was cancelled, stop processing
return ctx.Err()
@@ -205,7 +209,7 @@ func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string
}
// processRepository handles a single repository's PRs
-func processRepository(ctx context.Context, client *api.RESTClient, graphQlClient *api.GraphQLClient, spinner *Spinner, repo string) error {
+func processRepository(ctx context.Context, client *api.RESTClient, graphQlClient *api.GraphQLClient, repo string) error {
// Parse owner and repo name
parts := strings.Split(repo, "/")
if len(parts) != 2 {
From 49d53f65a8d7624031652eea4cafb3abc469e7da Mon Sep 17 00:00:00 2001
From: nobe4 <nobe4@users.noreply.github.com>
Date: Tue, 8 Apr 2025 10:14:19 +0200
Subject: [PATCH 5/8] feat(github): pull github-related logic into a new module
---
internal/cmd/combine_prs.go | 1 +
internal/cmd/match_criteria.go | 2 +
internal/cmd/root.go | 97 ++++++----------------------------
internal/github/github.go | 61 +++++++++++++++++++++
internal/github/repo.go | 21 ++++++++
5 files changed, 100 insertions(+), 82 deletions(-)
create mode 100644 internal/github/github.go
create mode 100644 internal/github/repo.go
diff --git a/internal/cmd/combine_prs.go b/internal/cmd/combine_prs.go
index cf0fa5d..4c4ba4a 100644
--- a/internal/cmd/combine_prs.go
+++ b/internal/cmd/combine_prs.go
@@ -1,3 +1,4 @@
+// @grant: this whole code should be moved to the github package.
package cmd
import (
diff --git a/internal/cmd/match_criteria.go b/internal/cmd/match_criteria.go
index 4f67b1f..5e00d36 100644
--- a/internal/cmd/match_criteria.go
+++ b/internal/cmd/match_criteria.go
@@ -119,6 +119,7 @@ func labelsMatchCriteria(prLabels []struct{ Name string }) bool {
return true
}
+// @grant: this should be moved to the github package
// GraphQL response structure for PR status info
type prStatusResponse struct {
Data struct {
@@ -142,6 +143,7 @@ type prStatusResponse struct {
} `json:"errors,omitempty"`
}
+// @grant: this should be moved to the github package
// GetPRStatusInfo fetches both CI status and approval status using GitHub's GraphQL API
func GetPRStatusInfo(ctx context.Context, graphQlClient *api.GraphQLClient, owner, repo string, prNumber int) (*prStatusResponse, error) {
// Check for context cancellation
diff --git a/internal/cmd/root.go b/internal/cmd/root.go
index 246b31d..285616f 100644
--- a/internal/cmd/root.go
+++ b/internal/cmd/root.go
@@ -4,12 +4,11 @@ import (
"context"
"errors"
"fmt"
- "strings"
-
- "github.com/cli/go-gh/v2/pkg/api"
+ "slices"
"github.com/spf13/cobra"
+ "github.com/github/gh-combine/internal/github"
"github.com/github/gh-combine/internal/version"
)
@@ -170,15 +169,9 @@ func runCombine(cmd *cobra.Command, args []string) error {
// executeCombineCommand performs the actual API calls and processing
func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string) error {
- // Create GitHub API client
- restClient, err := api.DefaultRESTClient()
- if err != nil {
- return fmt.Errorf("failed to get the default REST client: %w", err)
- }
-
- graphQlClient, err := api.DefaultGraphQLClient()
+ client, err := github.NewClient()
if err != nil {
- return fmt.Errorf("failed to create the default GraphQL Client : %w", err)
+ return fmt.Errorf("failed to create GitHub client: %w", err)
}
for _, repo := range repos {
@@ -194,7 +187,7 @@ func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string
Logger.Debug("Processing repository", "repo", repo)
// Process the repository
- if err := processRepository(ctx, restClient, graphQlClient, repo); err != nil {
+ if err := processRepository(ctx, client, graphQlClient, repo); err != nil {
if ctx.Err() != nil {
// If the context was cancelled, stop processing
return ctx.Err()
@@ -208,44 +201,10 @@ func executeCombineCommand(ctx context.Context, spinner *Spinner, repos []string
return nil
}
-// processRepository handles a single repository's PRs
-func processRepository(ctx context.Context, client *api.RESTClient, graphQlClient *api.GraphQLClient, repo string) error {
- // Parse owner and repo name
- parts := strings.Split(repo, "/")
- if len(parts) != 2 {
- return fmt.Errorf("invalid repository format: %s", repo)
- }
-
- owner := parts[0]
- repoName := parts[1]
-
- // Check for cancellation
- select {
- case <-ctx.Done():
- return ctx.Err()
- default:
- // Continue processing
- }
-
- // Get open PRs for the repository
- var pulls []struct {
- Number int
- Title string
- Head struct {
- Ref string
- }
- Base struct {
- Ref string
- SHA string
- }
- Labels []struct {
- Name string
- }
- }
-
- endpoint := fmt.Sprintf("repos/%s/%s/pulls?state=open", owner, repoName)
- if err := client.Get(endpoint, &pulls); err != nil {
- return err
+func processRepository(ctx context.Context, client github.Client, repo string) error {
+ openPRs, err := client.GetOpenPRs(ctx, repo)
+ if err != nil {
+ return fmt.Errorf("failed to get open PRs: %w", err)
}
// Check for cancellation again
@@ -256,21 +215,11 @@ func processRepository(ctx context.Context, client *api.RESTClient, graphQlClien
// Continue processing
}
- // Filter PRs based on criteria
- var matchedPRs []struct {
- Number int
- Title string
- Branch string
- Base string
- BaseSHA string
- }
-
- for _, pull := range pulls {
- branch := pull.Head.Ref
-
+ // @grant: filtering with DeleteFunc is _nice_.
+ filteredPRs := slices.DeleteFunc(openPRs, func(pr github.PR) bool {
// Check if PR matches all filtering criteria
- if !PrMatchesCriteria(branch, pull.Labels) {
- continue
+ if !PrMatchesCriteria(branch, pr.Labels) {
+ return true
}
// Check if PR meets additional requirements (CI, approval)
@@ -284,32 +233,16 @@ func processRepository(ctx context.Context, client *api.RESTClient, graphQlClien
// Skip this PR as it doesn't meet CI/approval requirements
continue
}
+ })
- matchedPRs = append(matchedPRs, struct {
- Number int
- Title string
- Branch string
- Base string
- BaseSHA string
- }{
- Number: pull.Number,
- Title: pull.Title,
- Branch: branch,
- Base: pull.Base.Ref,
- BaseSHA: pull.Base.SHA,
- })
- }
-
- // Check if we have enough PRs to combine
if len(matchedPRs) < minimum {
+ // @grant: shouldn't this be an error?
Logger.Debug("Not enough PRs match criteria", "repo", repo, "matched", len(matchedPRs), "required", minimum)
return nil
}
Logger.Debug("Matched PRs", "repo", repo, "count", len(matchedPRs))
- // If we get here, we have enough PRs to combine
-
// Combine the PRs
err := CombinePRs(ctx, graphQlClient, client, owner, repoName, matchedPRs)
if err != nil {
diff --git a/internal/github/github.go b/internal/github/github.go
new file mode 100644
index 0000000..61df7c9
--- /dev/null
+++ b/internal/github/github.go
@@ -0,0 +1,61 @@
+package github
+
+import (
+ "context"
+ "fmt"
+
+ "github.com/cli/go-gh/v2/pkg/api"
+)
+
+type Client struct {
+ REST *api.RESTClient
+ GraphQL *api.GraphQLClient
+}
+
+type PR struct {
+ Number int
+ Title string
+ Head struct {
+ Ref string
+ }
+ Base struct {
+ Ref string
+ SHA string
+ }
+ Labels []struct {
+ Name string
+ }
+}
+
+func New() (Client, error) {
+ var err error
+
+ c := Client{}
+
+ if c.REST, err = api.DefaultRESTClient(); err != nil {
+ return Client{}, fmt.Errorf("failed to get the default REST client: %w", err)
+ }
+
+ if c.GraphQL, err = api.DefaultGraphQLClient(); err != nil {
+ return Client{}, fmt.Errorf("failed to create the default GraphQL Client : %w", err)
+ }
+
+ return c, nil
+}
+
+func (c Client) GetOpenPRs(ctx context.Context, repo Repo) ([]PR, error) {
+ // Check for cancellation
+ select {
+ case <-ctx.Done():
+ return ctx.Err()
+ default:
+ // Continue processing
+ }
+
+ var pulls []PR
+
+ endpoint := fmt.Sprintf("repos/%s/%s/pulls?state=open", owner, repoName)
+ if err := c.REST.DoWithContext(ctx, "GET", endpoint, &pulls); err != nil {
+ return err
+ }
+}
diff --git a/internal/github/repo.go b/internal/github/repo.go
new file mode 100644
index 0000000..3f1715f
--- /dev/null
+++ b/internal/github/repo.go
@@ -0,0 +1,21 @@
+Package github
+
+
+type Repo struct {
+ Owner string
+ Repo string
+}
+
+// @grant: testing this should be pretty easy.
+// Make sure to create a custom error.
+func ParseRepo(repo string) (Repo, error) {
+ parts := strings.Split(repo, "/")
+ if len(parts) != 2 {
+ return Repo{}, fmt.Errorf("invalid repository format: %s", repo)
+ }
+
+ return Repo{
+ Owner: parts[0],
+ Repo: parts[1],
+ }, nil
+}
From 839569b59a1a48100ce90e01093cf8b99dc8da73 Mon Sep 17 00:00:00 2001
From: Grant Birkinbine <grant.birkinbine@gmail.com>
Date: Tue, 8 Apr 2025 16:01:28 -0700
Subject: [PATCH 6/8] Update internal/github/repo.go
---
internal/github/repo.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/internal/github/repo.go b/internal/github/repo.go
index 3f1715f..2427128 100644
--- a/internal/github/repo.go
+++ b/internal/github/repo.go
@@ -1,4 +1,4 @@
-Package github
+package github
type Repo struct {
From 5bbce843b784bf5a3429f3e90c43ffe90c1141ae Mon Sep 17 00:00:00 2001
From: GrantBirki <grant.birkinbine@gmail.com>
Date: Tue, 8 Apr 2025 16:03:26 -0700
Subject: [PATCH 7/8] run the linter
---
internal/github/repo.go | 1 -
1 file changed, 1 deletion(-)
diff --git a/internal/github/repo.go b/internal/github/repo.go
index 2427128..093caae 100644
--- a/internal/github/repo.go
+++ b/internal/github/repo.go
@@ -1,6 +1,5 @@
package github
-
type Repo struct {
Owner string
Repo string
From 9fa80b89341767045c5a99217b24c079c7b2cbb5 Mon Sep 17 00:00:00 2001
From: GrantBirki <grant.birkinbine@gmail.com>
Date: Tue, 8 Apr 2025 16:06:29 -0700
Subject: [PATCH 8/8] add missing imports
---
internal/github/repo.go | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/internal/github/repo.go b/internal/github/repo.go
index 093caae..ffb34cc 100644
--- a/internal/github/repo.go
+++ b/internal/github/repo.go
@@ -1,5 +1,10 @@
package github
+import (
+ "fmt"
+ "strings"
+)
+
type Repo struct {
Owner string
Repo string