From 48092d4f5e61661d09d812785bf6d9c74634ab38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 8 Oct 2019 13:02:35 +0200 Subject: [PATCH] [pr show] Avoid duplicate API requests --- commands/pr.go | 55 ++++++++++++++++++++++------------------ features/pr-show.feature | 23 ++++++----------- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/commands/pr.go b/commands/pr.go index 8e0637ce..ed9653f1 100644 --- a/commands/pr.go +++ b/commands/pr.go @@ -17,8 +17,8 @@ var ( Usage: ` pr list [-s ] [-h ] [-b ] [-o [-^]] [-f ] [-L ] pr checkout [] -pr show [-uc] [-h ] [-f ] -pr show [-uc] +pr show [-uc] [-f ] [-h ] +pr show [-uc] [-f ] `, Long: `Manage GitHub Pull Requests for the current repository. @@ -160,9 +160,15 @@ hub-issue(1), hub-pull-request(1), hub(1) } cmdShowPr = &Command{ - Key: "show", - Run: showPr, - Long: cmdPr.Long, + Key: "show", + Run: showPr, + KnownFlags: ` + -h, --head HEAD + -u, --url + -c, --copy + -f, --format FORMAT + --color +`, } ) @@ -277,9 +283,15 @@ func showPr(command *Command, args *Args) { baseProject, err := localRepo.MainProject() utils.Check(err) + host, err := github.CurrentConfig().PromptForHost(baseProject.Host) + utils.Check(err) + gh := github.NewClientWithHost(host) + words := args.Words() openUrl := "" prNumber := 0 + var pr *github.PullRequest + if len(words) > 0 { if prNumber, err = strconv.Atoi(words[0]); err == nil { openUrl = baseProject.WebURL("", "", fmt.Sprintf("pull/%d", prNumber)) @@ -287,36 +299,29 @@ func showPr(command *Command, args *Args) { utils.Check(fmt.Errorf("invalid pull request number: '%s'", words[0])) } } else { - pr, err := findCurrentPullRequest(localRepo, baseProject, args.Flag.Value("--head")) + pr, err = findCurrentPullRequest(localRepo, gh, baseProject, args.Flag.Value("--head")) utils.Check(err) openUrl = pr.HtmlUrl - prNumber = pr.Number } args.NoForward() if format := args.Flag.Value("--format"); format != "" { - host, err := github.CurrentConfig().PromptForHost(baseProject.Host) - utils.Check(err) - client := github.NewClientWithHost(host) - pr, err := client.PullRequest(baseProject, strconv.Itoa(prNumber)) - utils.Check(err) - - // ui.Println(pr.Number) + if pr == nil { + pr, err = gh.PullRequest(baseProject, strconv.Itoa(prNumber)) + utils.Check(err) + } colorize := colorizeOutput(args.Flag.HasReceived("--color"), args.Flag.Value("--color")) ui.Println(formatPullRequest(*pr, format, colorize)) - } else { - printUrl := args.Flag.Bool("--url") - copyUrl := args.Flag.Bool("--copy") - - printBrowseOrCopy(args, openUrl, !printUrl && !copyUrl, copyUrl) + return } -} -func findCurrentPullRequest(localRepo *github.GitHubRepo, baseProject *github.Project, headArg string) (*github.PullRequest, error) { - host, err := github.CurrentConfig().PromptForHost(baseProject.Host) - utils.Check(err) - gh := github.NewClientWithHost(host) + printUrl := args.Flag.Bool("--url") + copyUrl := args.Flag.Bool("--copy") + + printBrowseOrCopy(args, openUrl, !printUrl && !copyUrl, copyUrl) +} +func findCurrentPullRequest(localRepo *github.GitHubRepo, gh *github.Client, baseProject *github.Project, headArg string) (*github.PullRequest, error) { filterParams := map[string]interface{}{ "state": "open", } @@ -332,7 +337,7 @@ func findCurrentPullRequest(localRepo *github.GitHubRepo, baseProject *github.Pr utils.Check(err) if headBranch, headProject, err := findPushTarget(currentBranch); err == nil { headWithOwner = fmt.Sprintf("%s:%s", headProject.Owner, headBranch.ShortName()) - } else if headProject, err := deducePushTarget(currentBranch, host.User); err == nil { + } else if headProject, err := deducePushTarget(currentBranch, gh.Host.User); err == nil { headWithOwner = fmt.Sprintf("%s:%s", headProject.Owner, currentBranch.ShortName()) } else { headWithOwner = fmt.Sprintf("%s:%s", baseProject.Owner, currentBranch.ShortName()) diff --git a/features/pr-show.feature b/features/pr-show.feature index 5b69eb02..f62d1e33 100644 --- a/features/pr-show.feature +++ b/features/pr-show.feature @@ -44,17 +44,8 @@ Feature: hub pr show get('/repos/ashemesh/hub/pulls'){ assert :state => "open", :head => "ashemesh:topic" - json [ - { - :html_url => "https://github.com/ashemesh/hub/pull/102", - :number => 102 - }, - ] - } - - get('/repos/ashemesh/hub/pulls/102') { - json :number => 999, - :title => "First", + json [{ + :number => 102, :state => "open", :base => { :ref => "master", @@ -69,14 +60,16 @@ Feature: hub pr show :requested_teams => [ { :slug => "troopers" }, { :slug => "cantina-band" }, - ] + ], + :html_url => "https://github.com/ashemesh/hub/pull/102", + }] } """ When I successfully run `hub pr show -f "%sC%>(8)%i %rs%n"` Then "open https://github.com/ashemesh/hub/pull/102" should not be run And the output should contain exactly: """ - #999 rey, github/troopers, github/cantina-band\n + #102 rey, github/troopers, github/cantina-band\n """ Scenario: Current branch in fork @@ -192,7 +185,7 @@ Feature: hub pr show Given the GitHub API server: """ get('/repos/ashemesh/hub/pulls/102') { - json :number => 999, + json :number => 102, :title => "First", :state => "open", :base => { @@ -215,7 +208,7 @@ Feature: hub pr show Then "open https://github.com/ashemesh/hub/pull/102" should not be run And the output should contain exactly: """ - #999 rey, github/troopers, github/cantina-band\n + #102 rey, github/troopers, github/cantina-band\n """ Scenario: Show pull request by invalid number -- GitLab