Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent PR lookup interface #1020

Open
wants to merge 8 commits into
base: trunk
from
Open

Consistent PR lookup interface #1020

wants to merge 8 commits into from

Conversation

@probablycorey
Copy link
Collaborator

probablycorey commented May 27, 2020

We've got a few ways to get a PR object right now, and they are all slightly different and require a lot of boilerplate code. This creates a consistent interface for that lookup. It might sound familiar because I tried it before in #926 but this time it will work!

The plan

This is going to be a two step process.

  1. Get the commands using the same pr lookup interface prFromArgs. This will look at the commands args to find the PR via a number, url, branch name or whatever the current branch of the existing git repo is. This is what I solved in this PR.

  2. Add an argument to prFromArgs that lets you pick what type of PR response you want back. This was mislav's magical reflection idea that we got working in https://github.com/cli/cli/compare/pr-get-query

Things I considered while writing this.

  • I pass the args string to prFromArgs because if passed the string itself like args[0] we'd get a panic when there are no args or we'd need a conditional before each call. This seemed annoying and passing the args array worked out well enough.
  • This does create an extra api in some commands because previously we only needed the PR number. But this is not common and I feel like it gives us the extra benefit of verifying that the PR exists.
@probablycorey probablycorey self-assigned this May 27, 2020
@probablycorey probablycorey requested a review from cli/code-reviewers May 27, 2020
@probablycorey probablycorey added this to Needs review 🤔 in The GitHub CLI May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
The GitHub CLI
  
Needs review 🤔
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.