Skip to content
This repository has been archived by the owner on Nov 4, 2022. It is now read-only.

Display pull request mergeable status on "gh pr" list #305

Closed
mdelapenya opened this issue Sep 9, 2014 · 10 comments
Closed

Display pull request mergeable status on "gh pr" list #305

mdelapenya opened this issue Sep 9, 2014 · 10 comments
Assignees

Comments

@mdelapenya
Copy link
Contributor

Sometimes reviewers don't even visit github to see their pending pull requests list, because it's easier executing:

gh pr

I'd like to see the mergeable status of the pull request, so that reviewer can see if the pull is good to merge or not.

@eduardolundgren
Copy link
Member

👍

@zenorocha zenorocha self-assigned this Sep 9, 2014
@zenorocha
Copy link
Contributor

Nice idea @mdelapenya!

The good news is that a mergeable info can be retrieved from the API.

https://developer.github.com/v3/pulls/#mergability

The bad news is that it's only available when you fetch each PR separately and not when you list all of them like we do nowadays.

@mdelapenya
Copy link
Contributor Author

And how about adding a command to the gh pr, like:

gh pr --info 323
or
gh pr -I 323

which retrieves one pull request at a time, displaying its comments, mergeable status, or/and whatever a reviewer will need before starting the review?

That will follow the API for one single pull

@zenorocha zenorocha modified the milestone: 1.9.4 Oct 20, 2014
@zenorocha zenorocha modified the milestone: 1.9.4 Jan 6, 2015
@henvic henvic assigned henvic and unassigned zenorocha Mar 12, 2015
@henvic
Copy link
Member

henvic commented Mar 13, 2015

Nice idea. I've made some modifications and it already works, but it's not ready for release yet.

@mdelapenya, can you take a look at https://developer.github.com/v3/pulls/#get-a-single-pull-request and please tell me what else you'd find it useful for the --info command to give? Should it show always everything or the --details flag should add something more?

Best regards,
Henrique.

@mdelapenya
Copy link
Contributor Author

Let's start with the mergeable status and the comments on the pull. I guess we can improve later

Thanks @henvic!!

@mdelapenya
Copy link
Contributor Author

Any update on this? :)

@henvic
Copy link
Member

henvic commented Jun 4, 2015

@mdelapenya I am planning to do this on the weekend.

henvic added a commit that referenced this issue Jun 9, 2015
@henvic henvic closed this as completed in f20c8e8 Jun 9, 2015
@henvic
Copy link
Member

henvic commented Jun 9, 2015

@mdelapenya can you please try the new --info command?

I have not added comments just yet, but will pretty soon.
Sorry for the delay, I couldn't do this until today.

For now --list isn't showing if the pull request is mergeable because it would make --list painfully slow. If GitHub starts to provide this info on their public API I will update. Is that ok?

I have contacted GitHub to ask them if there's any chance they'd add this on the list API method.

@henvic
Copy link
Member

henvic commented Jun 17, 2015

Hi Henrique,

Thanks for expressing interest in this -- I'll mention it to the team to consider, but I don't expect it to happen in the near future. Computing the mergeability is a bit expensive, so that's why it's included only when you request a specific pull request (mergeability is computed on demand and in the background):

https://developer.github.com/v3/#summary-representations

https://developer.github.com/v3/pulls/#mergability

I hope this answers your question, but let me know if there's anything else.

Cheers,
Ivan

Just wondering: is there any chance the mergeable flag will be added to the API's list method?

Right now it's only exposed with the 'Get a single pull request' method:
GET /repos/:owner/:repo/pulls/:number

And I have some users claiming it'd be useful for them to see what pull requests are mergeable when listing with this tool I maintain.
#305

Best regards,
Henrique.

@mdelapenya
Copy link
Contributor Author

Hi Henrique, thanks for the follow-up of this ticket!

I understand the problem, so for me, current --info command could be enough

gh pr 445 --info
#445 LPS-55528 Not needed @csierra (20 hours ago)
warning: Not mergeable (dirty)

Maybe we could follow a color code for mergeable status, just the same in the real UI.

What do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants