Skip to content

Commit

Permalink
refactor: Abstract GitHub client away (#140)
Browse files Browse the repository at this point in the history 
Adds a layer between GitHub client and business logic currently referred
to as "forge" with the idea that we may support non-GitHub code forges
in the future.

The abstraction layer refers to PRs as Changes to further disconnect
from GitHub-specific terminology.

This also fixes a previously known bug where `GITHUB_GIT_URL`
was not actually accounted for when matching the remote URL.
In this change, that's renamed to `GITHUB_URL`, and is actually used
when determining the GitHub repository.

The internal/gh package has been merged into this new package
while we're at it.
  • Loading branch information
@abhinav
abhinav authored Jun 1, 2024
1 parent 3cb0a73 commit 9331441
Show file tree
Hide file tree
Showing 26 changed files with 468 additions and 207 deletions.
3 changes: 3 additions & 0 deletions .changes/unreleased/Fixed-20240601-151223.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Fixed
body: GitHub URL detection now respects non-standard URLs like GHES set via `$GITHUB_URL`.
time: 2024-06-01T15:12:23.588802-07:00
78 changes: 37 additions & 41 deletions branch_submit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import (
"strings"

"github.com/charmbracelet/log"
"github.com/google/go-github/v61/github"
"go.abhg.dev/gs/internal/forge/github"
"go.abhg.dev/gs/internal/git"
"go.abhg.dev/gs/internal/must"
"go.abhg.dev/gs/internal/spice"
"go.abhg.dev/gs/internal/spice/state"
"go.abhg.dev/gs/internal/text"
"go.abhg.dev/gs/internal/ui"
"golang.org/x/oauth2"
)

type branchSubmitCmd struct {
Expand Down Expand Up @@ -52,7 +51,7 @@ func (cmd *branchSubmitCmd) Run(
ctx context.Context,
log *log.Logger,
opts *globalOptions,
tokenSource oauth2.TokenSource,
ghBuilder *github.Builder,
) error {
repo, err := git.Open(ctx, ".", git.OpenOptions{
Log: log,
Expand Down Expand Up @@ -108,25 +107,21 @@ func (cmd *branchSubmitCmd) Run(
return err
}

ghrepo, err := ensureGitHubRepo(ctx, log, repo, remote)
forge, err := ensureGitHubForge(ctx, log, ghBuilder, repo, remote)
if err != nil {
return err
}

gh, err := newGitHubClient(ctx, tokenSource, opts)
if err != nil {
return fmt.Errorf("create GitHub client: %w", err)
}
pulls, _, err := gh.PullRequests.List(ctx, ghrepo.Owner, ghrepo.Name, &github.PullRequestListOptions{
State: "open",
Head: ghrepo.Owner + ":" + upstreamBranch,
// Don't filter by base -- we may need to update it.
changes, err := forge.ListChanges(ctx, github.ListChangesOptions{
State: "open",
Branch: upstreamBranch,
// We don't filter by base here as that may be out of date.
})
if err != nil {
return fmt.Errorf("list pull requests: %w", err)
return fmt.Errorf("list changes: %w", err)
}

switch len(pulls) {
switch len(changes) {
case 0:
if cmd.DryRun {
log.Infof("WOULD create a pull request for %s", cmd.Name)
Expand Down Expand Up @@ -251,76 +246,77 @@ func (cmd *branchSubmitCmd) Run(
log.Warn("Could not set upstream", "branch", cmd.Name, "remote", remote, "error", err)
}

pull, _, err := gh.PullRequests.Create(ctx, ghrepo.Owner, ghrepo.Name, &github.NewPullRequest{
Title: &cmd.Title,
Body: &cmd.Body,
Head: &cmd.Name,
Base: &branch.Base,
Draft: &cmd.Draft,
result, err := forge.SubmitChange(ctx, github.SubmitChangeRequest{
Subject: cmd.Title,
Body: cmd.Body,
Head: cmd.Name,
Base: branch.Base,
Draft: cmd.Draft,
})
if err != nil {
return fmt.Errorf("create pull request: %w", err)
return fmt.Errorf("create change: %w", err)
}
upsert.PR = pull.GetNumber()
upsert.PR = int(result.ID)

log.Infof("Created #%d: %s", pull.GetNumber(), pull.GetHTMLURL())
log.Infof("Created %v: %s", result.ID, result.URL)

case 1:
// Check base and HEAD are up-to-date.
pull := pulls[0]
pull := changes[0]
var updates []string
if pull.Head.GetSHA() != commitHash.String() {
if pull.HeadHash != commitHash {
updates = append(updates, "push branch")
}
if pull.Base.GetRef() != branch.Base {
if pull.BaseName != branch.Base {
updates = append(updates, "set base to "+branch.Base)
}
if pull.GetDraft() != cmd.Draft {
if pull.Draft != cmd.Draft {
updates = append(updates, "set draft to "+fmt.Sprint(cmd.Draft))
}

if len(updates) == 0 {
log.Infof("Pull request #%d is up-to-date", pull.GetNumber())
log.Infof("Pull request %v is up-to-date", pull.ID)
return nil
}

if cmd.DryRun {
log.Infof("WOULD update PR #%d:", pull.GetNumber())
log.Infof("WOULD update PR %v:", pull.ID)
for _, update := range updates {
log.Infof(" - %s", update)
}
return nil
}

if pull.Head.GetSHA() != commitHash.String() {
if pull.HeadHash != commitHash {
err := repo.Push(ctx, git.PushOptions{
Remote: remote,
Refspec: git.Refspec(
commitHash.String() + ":refs/heads/" + upstreamBranch,
),
// Force push, but only if the ref is exactly
// where we think it is.
ForceWithLease: cmd.Name + ":" + pull.Head.GetSHA(),
ForceWithLease: cmd.Name + ":" + pull.HeadHash.String(),
})
if err != nil {
log.Error("Branch may have been updated by someone else.")
return fmt.Errorf("push branch: %w", err)
}
}

if pull.Base.GetRef() != branch.Base || pull.GetDraft() != cmd.Draft {
_, _, err := gh.PullRequests.Edit(ctx, ghrepo.Owner, ghrepo.Name, pull.GetNumber(), &github.PullRequest{
Base: &github.PullRequestBranch{
Ref: &branch.Base,
},
Draft: &cmd.Draft,
})
if err != nil {
return fmt.Errorf("update PR #%d: %w", pull.GetNumber(), err)
if pull.BaseName != branch.Base || pull.Draft != cmd.Draft {
opts := github.EditChangeOptions{
Base: branch.Base,
}
if pull.Draft != cmd.Draft {
opts.Draft = &cmd.Draft
}

if err := forge.EditChange(ctx, pull.ID, opts); err != nil {
return fmt.Errorf("edit PR %v: %w", pull.ID, err)
}
}

log.Infof("Updated #%d: %s", pull.GetNumber(), pull.GetHTMLURL())
log.Infof("Updated %v: %s", pull.ID, pull.URL)

default:
// TODO: add a --pr flag to allow picking a PR?
Expand Down
6 changes: 3 additions & 3 deletions downstack_submit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
"slices"

"github.com/charmbracelet/log"
"go.abhg.dev/gs/internal/forge/github"
"go.abhg.dev/gs/internal/git"
"go.abhg.dev/gs/internal/must"
"go.abhg.dev/gs/internal/spice"
"go.abhg.dev/gs/internal/text"
"golang.org/x/oauth2"
)

type downstackSubmitCmd struct {
Expand All @@ -37,7 +37,7 @@ func (cmd *downstackSubmitCmd) Run(
ctx context.Context,
log *log.Logger,
opts *globalOptions,
tokenSource oauth2.TokenSource,
ghBuilder *github.Builder,
) error {
repo, err := git.Open(ctx, ".", git.OpenOptions{
Log: log,
Expand Down Expand Up @@ -79,7 +79,7 @@ func (cmd *downstackSubmitCmd) Run(
DryRun: cmd.DryRun,
Fill: cmd.Fill,
Name: downstack,
}).Run(ctx, log, opts, tokenSource)
}).Run(ctx, log, opts, ghBuilder)
if err != nil {
return fmt.Errorf("submit %v: %w", downstack, err)
}
Expand Down
38 changes: 13 additions & 25 deletions gh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,35 @@ package main

import (
"context"
"errors"
"fmt"

"github.com/charmbracelet/log"
"github.com/google/go-github/v61/github"
"go.abhg.dev/gs/internal/gh"
"go.abhg.dev/gs/internal/forge/github"
"go.abhg.dev/gs/internal/git"
"golang.org/x/oauth2"
)

func ensureGitHubRepo(
func ensureGitHubForge(
ctx context.Context,
log *log.Logger,
builder *github.Builder,
repo *git.Repository,
remote string,
) (gh.RepoInfo, error) {
) (*github.Forge, error) {
remoteURL, err := repo.RemoteURL(ctx, remote)
if err != nil {
return gh.RepoInfo{}, fmt.Errorf("get remote URL: %w", err)
return nil, fmt.Errorf("get remote URL: %w", err)
}

// TODO: Take GITHUB_GIT_URL into account for ParseRepoInfo.
ghrepo, err := gh.ParseRepoInfo(remoteURL)
forge, err := builder.New(ctx, remoteURL)
if err != nil {
log.Error("Could not guess GitHub repository from remote URL", "url", remoteURL)
log.Error("Are you sure the remote is a GitHub repository?")
return gh.RepoInfo{}, err
}

return ghrepo, nil
}

// TODO: move this into gh package
func newGitHubClient(ctx context.Context, tokenSource oauth2.TokenSource, opts *globalOptions) (*github.Client, error) {
gh := github.NewClient(oauth2.NewClient(ctx, tokenSource))
if opts.GithubAPIURL != "" {
var err error
gh, err = gh.WithEnterpriseURLs(opts.GithubAPIURL, gh.UploadURL.String())
if err != nil {
return nil, fmt.Errorf("set GitHub API URL: %w", err)
if errors.Is(err, github.ErrUnsupportedURL) {
log.Error("Could not guess GitHub repository from remote URL", "url", remoteURL)
log.Error("Are you sure the remote is a GitHub repository?")
return nil, err
}
return nil, fmt.Errorf("build GitHub Forge: %w", err)
}

return gh, nil
return forge, nil
}
Loading

0 comments on commit 9331441

Please sign in to comment.