Reuse most logic betwen `checkout` and `pr` commands

This commit is contained in:
Mislav Marohnić 2017-04-08 22:14:22 +02:00
Родитель e198d64a73
Коммит 6be9d54fc2
4 изменённых файлов: 43 добавлений и 325 удалений

Просмотреть файл

@ -31,17 +31,10 @@ func init() {
}
func checkout(command *Command, args *Args) {
if !args.IsParamsEmpty() {
err := transformCheckoutArgs(args)
utils.Check(err)
}
}
func transformCheckoutArgs(args *Args) error {
words := args.Words()
if len(words) == 0 {
return nil
return
}
checkoutURL := words[0]
@ -53,40 +46,42 @@ func transformCheckoutArgs(args *Args) error {
url, err := github.ParseURL(checkoutURL)
if err != nil {
// not a valid GitHub URL
return nil
return
}
pullURLRegex := regexp.MustCompile("^pull/(\\d+)")
projectPath := url.ProjectPath()
if !pullURLRegex.MatchString(projectPath) {
// not a valid PR URL
return nil
return
}
err = sanitizeCheckoutFlags(args)
if err != nil {
return err
}
utils.Check(err)
id := pullURLRegex.FindStringSubmatch(projectPath)[1]
gh := github.NewClient(url.Project.Host)
pullRequest, err := gh.PullRequest(url.Project, id)
if err != nil {
return err
}
utils.Check(err)
newArgs, err := transformCheckoutArgs(args, pullRequest, newBranchName)
utils.Check(err)
if idx := args.IndexOfParam(newBranchName); idx >= 0 {
args.RemoveParam(idx)
}
replaceCheckoutParam(args, checkoutURL, newArgs...)
}
func transformCheckoutArgs(args *Args, pullRequest *github.PullRequest, newBranchName string) (newArgs []string, err error) {
repo, err := github.LocalRepo()
if err != nil {
return err
return
}
baseRemote, err := repo.RemoteForRepo(pullRequest.Base.Repo)
if err != nil {
return err
return
}
var headRemote *github.Remote
@ -96,8 +91,6 @@ func transformCheckoutArgs(args *Args) error {
headRemote, _ = repo.RemoteForRepo(pullRequest.Head.Repo)
}
var newArgs []string
if headRemote != nil {
if newBranchName == "" {
newBranchName = pullRequest.Head.Ref
@ -114,22 +107,23 @@ func transformCheckoutArgs(args *Args) error {
} else {
if newBranchName == "" {
if pullRequest.Head.Repo == nil {
newBranchName = fmt.Sprintf("pr-%s", id)
newBranchName = fmt.Sprintf("pr-%d", pullRequest.Number)
} else {
newBranchName = fmt.Sprintf("%s-%s", pullRequest.Head.Repo.Owner.Login, pullRequest.Head.Ref)
}
}
newArgs = append(newArgs, newBranchName)
ref := fmt.Sprintf("refs/pull/%s/head", id)
ref := fmt.Sprintf("refs/pull/%d/head", pullRequest.Number)
args.Before("git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName))
remote := baseRemote.Name
mergeRef := ref
if pullRequest.MaintainerCanModify && pullRequest.Head.Repo != nil {
project, err := github.NewProjectFromRepo(pullRequest.Head.Repo)
var project *github.Project
project, err = github.NewProjectFromRepo(pullRequest.Head.Repo)
if err != nil {
return err
return
}
remote = project.GitURL("", "", true)
@ -138,8 +132,7 @@ func transformCheckoutArgs(args *Args) error {
args.Before("git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote)
args.Before("git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef)
}
replaceCheckoutParam(args, checkoutURL, newArgs...)
return nil
return
}
func sanitizeCheckoutFlags(args *Args) error {

Просмотреть файл

@ -3,10 +3,8 @@ package commands
import (
"fmt"
"os"
"regexp"
"strconv"
"github.com/github/hub/git"
"github.com/github/hub/github"
"github.com/github/hub/utils"
)
@ -45,11 +43,16 @@ func printHelp(command *Command, args *Args) {
}
func checkoutPr(command *Command, args *Args) {
if args.ParamsSize() < 1 || args.ParamsSize() > 2 {
utils.Check(fmt.Errorf("Error: Expected one or two arguments, got %d", args.ParamsSize()))
words := args.Words()
var newBranchName string
if len(words) == 0 {
utils.Check(fmt.Errorf("Error: No pull request number given"))
} else if len(words) > 1 {
newBranchName = words[1]
}
prNumberString := args.GetParam(0)
prNumberString := words[0]
_, err := strconv.Atoi(prNumberString)
utils.Check(err)
@ -64,116 +67,8 @@ func checkoutPr(command *Command, args *Args) {
pr, err := client.PullRequest(baseProject, prNumberString)
utils.Check(err)
// Args here are: "git pr 77" or "git pr 77 new-branch-name"
if args.ParamsSize() == 1 {
args.Replace(args.Executable, "checkout", pr.HtmlUrl)
} else {
args.Replace(args.Executable, "checkout", pr.HtmlUrl, args.GetParam(1))
}
// Call into the checkout code which already provides the functionality we're
// after
err = transformPrArgs(args, pr)
newArgs, err := transformCheckoutArgs(args, pr, newBranchName)
utils.Check(err)
}
func transformPrArgs(args *Args, pullRequest *github.PullRequest) error {
// This function initially copied from checkout.go:transformCheckoutArgs()
words := args.Words()
if len(words) == 0 {
return nil
}
checkoutURL := words[0]
var newBranchName string
if len(words) > 1 {
newBranchName = words[1]
}
url, err := github.ParseURL(checkoutURL)
if err != nil {
// not a valid GitHub URL
return nil
}
pullURLRegex := regexp.MustCompile("^pull/(\\d+)")
projectPath := url.ProjectPath()
if !pullURLRegex.MatchString(projectPath) {
// not a valid PR URL
return nil
}
err = sanitizeCheckoutFlags(args)
if err != nil {
return err
}
id := pullURLRegex.FindStringSubmatch(projectPath)[1]
if idx := args.IndexOfParam(newBranchName); idx >= 0 {
args.RemoveParam(idx)
}
repo, err := github.LocalRepo()
if err != nil {
return err
}
baseRemote, err := repo.RemoteForRepo(pullRequest.Base.Repo)
if err != nil {
return err
}
var headRemote *github.Remote
if pullRequest.IsSameRepo() {
headRemote = baseRemote
} else if pullRequest.Head.Repo != nil {
headRemote, _ = repo.RemoteForRepo(pullRequest.Head.Repo)
}
var newArgs []string
if headRemote != nil {
if newBranchName == "" {
newBranchName = pullRequest.Head.Ref
}
remoteBranch := fmt.Sprintf("%s/%s", headRemote.Name, pullRequest.Head.Ref)
refSpec := fmt.Sprintf("+refs/heads/%s:refs/remotes/%s", pullRequest.Head.Ref, remoteBranch)
if git.HasFile("refs", "heads", newBranchName) {
newArgs = append(newArgs, newBranchName)
args.After("git", "merge", "--ff-only", fmt.Sprintf("refs/remotes/%s", remoteBranch))
} else {
newArgs = append(newArgs, "-b", newBranchName, "--track", remoteBranch)
}
args.Before("git", "fetch", headRemote.Name, refSpec)
} else {
if newBranchName == "" {
if pullRequest.Head.Repo == nil {
newBranchName = fmt.Sprintf("pr-%s", id)
} else {
newBranchName = fmt.Sprintf("%s-%s", pullRequest.Head.Repo.Owner.Login, pullRequest.Head.Ref)
}
}
newArgs = append(newArgs, newBranchName)
ref := fmt.Sprintf("refs/pull/%s/head", id)
args.Before("git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName))
remote := baseRemote.Name
mergeRef := ref
if pullRequest.MaintainerCanModify && pullRequest.Head.Repo != nil {
project, projectErr := github.NewProjectFromRepo(pullRequest.Head.Repo)
if projectErr != nil {
return projectErr
}
remote = project.GitURL("", "", true)
mergeRef = fmt.Sprintf("refs/heads/%s", pullRequest.Head.Ref)
}
args.Before("git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote)
args.Before("git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef)
}
replaceCheckoutParam(args, checkoutURL, newArgs...)
return nil
args.Replace(args.Executable, "checkout", newArgs...)
}

Просмотреть файл

@ -12,7 +12,7 @@ Feature: hub checkout <PULLREQ-URL>
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
@ -37,7 +37,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mislav/jekyll/pulls/77') {
json :base => {
json :number => 77, :base => {
:repo => {
:name => 'jekyll',
:html_url => 'https://github.com/mislav/jekyll',
@ -57,7 +57,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
@ -81,7 +81,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
@ -104,7 +104,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
@ -127,7 +127,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => nil
}, :base => {
@ -148,7 +148,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
@ -173,7 +173,7 @@ Feature: hub checkout <PULLREQ-URL>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
@ -201,7 +201,7 @@ Feature: hub checkout <PULLREQ-URL>
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
@ -228,7 +228,7 @@ Feature: hub checkout <PULLREQ-URL>
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },

Просмотреть файл

@ -7,8 +7,7 @@ Feature: hub pr checkout <PULLREQ-NUMBER>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
@ -31,21 +30,11 @@ Feature: hub pr checkout <PULLREQ-NUMBER>
And "git checkout mislav-fixes" should be run
And "mislav-fixes" should merge "refs/pull/77/head" from remote "origin"
# Scenario: No matching remotes for pull request base
#
# This scenario was removed.
#
# With "hub checkout" you had to write the complete PR URL, which made it
# possible to type the wrong owner/repo there. This check covered that case.
#
# With "hub pr checkout" you only enter the PR number and you can't mess up
# that way any more. Thus, this test lost its meaning and was removed.
Scenario: Custom name for new branch
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
@ -71,7 +60,7 @@ Feature: hub pr checkout <PULLREQ-NUMBER>
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
json :number => 77, :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
@ -90,162 +79,3 @@ Feature: hub pr checkout <PULLREQ-NUMBER>
When I run `hub pr checkout 77`
Then "git fetch origin +refs/heads/fixes:refs/remotes/origin/fixes" should be run
And "git checkout -b fixes --track origin/fixes" should be run
Scenario: Same-repo with custom branch name
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
:ref => "fixes",
:repo => {
:name => "jekyll",
:owner => { :login => "mojombo" },
}
}, :base => {
:repo => {
:name => "jekyll",
:html_url => "https://github.com/mojombo/jekyll",
:owner => { :login => "mojombo" },
}
},
:html_url => 'https://github.com/mojombo/jekyll/pull/77'
}
"""
When I run `hub pr checkout 77 mycustombranch`
Then "git fetch origin +refs/heads/fixes:refs/remotes/origin/fixes" should be run
And "git checkout -b mycustombranch --track origin/fixes" should be run
Scenario: Unavailable fork
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
:ref => "fixes",
:repo => nil
}, :base => {
:repo => {
:name => "jekyll",
:html_url => "https://github.com/mojombo/jekyll",
:owner => { :login => "mojombo" },
}
},
:html_url => "https://github.com/mojombo/jekyll/pull/77"
}
"""
When I run `hub pr checkout 77`
Then "git fetch origin refs/pull/77/head:pr-77" should be run
And "git checkout pr-77" should be run
And "pr-77" should merge "refs/pull/77/head" from remote "origin"
Scenario: Reuse existing remote for head branch
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
:name => "jekyll",
:private => false
}
}, :base => {
:repo => {
:name => 'jekyll',
:html_url => 'https://github.com/mojombo/jekyll',
:owner => { :login => "mojombo" },
}
},
:html_url => "https://github.com/mojombo/jekyll/pull/77"
}
"""
And the "mislav" remote has url "git://github.com/mislav/jekyll.git"
When I run `hub pr checkout 77`
Then "git fetch mislav +refs/heads/fixes:refs/remotes/mislav/fixes" should be run
And "git checkout -b fixes --track mislav/fixes" should be run
Scenario: Reuse existing remote and branch
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
json :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
:name => "jekyll",
:private => false
}
}, :base => {
:repo => {
:name => 'jekyll',
:html_url => 'https://github.com/mojombo/jekyll',
:owner => { :login => "mojombo" },
}
},
:html_url => "https://github.com/mojombo/jekyll/pull/77"
}
"""
And the "mislav" remote has url "git://github.com/mislav/jekyll.git"
And I am on the "fixes" branch
When I run `hub pr checkout 77`
Then "git fetch mislav +refs/heads/fixes:refs/remotes/mislav/fixes" should be run
And "git checkout fixes" should be run
And "git merge --ff-only refs/remotes/mislav/fixes" should be run
Scenario: Modifiable fork
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
:name => "jekyll",
:html_url => "https://github.com/mislav/jekyll.git",
:private => false
},
}, :base => {
:repo => {
:name => 'jekyll',
:html_url => 'https://github.com/mojombo/jekyll',
:owner => { :login => "mojombo" },
}
},
:maintainer_can_modify => true,
:html_url => "https://github.com/mojombo/jekyll/pull/77"
}
"""
When I run `hub pr checkout 77`
Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run
And "git checkout mislav-fixes" should be run
And "mislav-fixes" should merge "refs/heads/fixes" from remote "git@github.com:mislav/jekyll.git"
Scenario: Modifiable fork with HTTPS
Given the GitHub API server:
"""
get('/repos/mojombo/jekyll/pulls/77') {
halt 406 unless request.env['HTTP_ACCEPT'] == 'application/vnd.github.v3+json;charset=utf-8'
json :head => {
:ref => "fixes",
:repo => {
:owner => { :login => "mislav" },
:name => "jekyll",
:html_url => "https://github.com/mislav/jekyll.git",
:private => false
},
}, :base => {
:repo => {
:name => 'jekyll',
:html_url => 'https://github.com/mojombo/jekyll',
:owner => { :login => "mojombo" },
}
},
:maintainer_can_modify => true,
:html_url => "https://github.com/mojombo/jekyll/pull/77"
}
"""
And HTTPS is preferred
When I run `hub pr checkout 77`
Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run
And "git checkout mislav-fixes" should be run
And "mislav-fixes" should merge "refs/heads/fixes" from remote "https://github.com/mislav/jekyll.git"