hub
hub copied to clipboard
Checks status hub pr list
Fixes #2193
- Add check status for pull requests list on the command of
hub pr list
- Add
query.go
for constructing the particular GraphQL query usinggraphb
. - Screenshot attached below.
I am so sorry beforehand if I got the design wrong in any way. Looking forward to the review! :slightly_smiling_face:
Hi @mislav , truly sorry for a really late reply :disappointed: . First of all, I just want to say that I am blown away and thank you very much for the really kind reply :slightly_smiling_face: .
Yeah, I noticed it before that there are 2 ways of getting status which the first is Status and the second is Check, which is represented in the GraphQL query that you mentioned in the issue before, right? Thus if it's okay with you, I have several questions on this.
- Wouldn't all GitHub repositories with CI installed will always have Status value but not always have Checks status value from GitHub Apps installed? and,
- I thought that if there are GitHub apps installed and it would return Checks value, the value of the Checks will always be the same and reflected by the Status value? So that it's sufficient to retrieve the Status value only.
- And if we are going to retrieve both of the two values from Status and also Checks, which value are we going to use? In case it would return different value.
I am really sorry beforehand if it's because I have a wrong understanding on this Status and Checks. I will reply on the remaining reviews below. Looking forward to your reply and hope you have a great week ahead! :slightly_smiling_face:
I am really sorry beforehand if it's because I have a wrong understanding on this Status and Checks.
No need to apologize, this stuff isn't really easy, and our documentation doesn't explain the differences between Statuses and Checks very well. Our GraphQL documentation for Checks is also lacking since that API is in preview phase.
This is what it all comes down to:
- Statuses were the original CI for GitHub
- Checks are the new generation of Statuses and come from GitHub Apps
- A repository can be configured with any combination of Checks and Statuses
- Checks right now are complex to fetch over GraphQL.
For the simplicity of this PR, I would like you to focus on Statuses only, and I can then add Checks support as follow-up. 🙇
@joshuabezaleel For an example of how to perform GraphQL queries in hub, here is a PR that adds some simple queries in a way that doesn't require an external library https://github.com/github/hub/pull/2363
I have pushed some changes, mainly to:
- compose GraphQL requests directly without the
graphb
library - this utilizes theClient.GraphQL
function that is new in master - restore default
--format
to original - I feel like this feature is still premature to be enabled by default - avoid fetching statuses via GraphQL unless
%cs
/%cC
were explicitly specified via--format
- support
--limit
values > 100
@joshuabezaleel Please review! 🙇
I'm still not sure whether this feature is ready to merge, though:
- It only fetches Statuses so far and not Checks (to reiterate: I will take on implementing Checks when I find time, so you don't have to worry about that);
- It fetches pull requests double: once over REST (like before) and another time over GraphQL (to fetch Statuses). Ideally it would only fetch them once, so maybe we could switch to GraphQL entirely for the case when Statuses are needed.
-
--sort long-running
is supported over REST API but not over GraphQL.
Will be this continued, since now the most attention will go to the CLI instead?
Are there any updates regarding this PR? It needs to be rebased.
@XVilka @joshuabezaleel The GitHub API in the meantime had some updates to make querying Checks + Statuses easier! I plan to update this PR incorporating those changes when I find time.
I would really love this feature. Is anyone working on this still?
@dawsbot I haven't worked on it since my last comment, but since then there is a new GraphQL statusCheckRollup
API that combines information about both Checks and Statuses and their resolution, so there's less work on the client to assemble this data. I plan to adopt it in this PR when I find time. Or, in the meantime, anyone is welcome to base their work off of my branch! 🙇