hub icon indicating copy to clipboard operation
hub copied to clipboard

Checks status hub pr list

Open joshuabezaleel opened this issue 5 years ago • 9 comments

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 using graphb.
  • Screenshot attached below.

image

I am so sorry beforehand if I got the design wrong in any way. Looking forward to the review! :slightly_smiling_face:

joshuabezaleel avatar Nov 15 '19 06:11 joshuabezaleel

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.

  1. 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,
  2. 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.
  3. 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:

joshuabezaleel avatar Dec 04 '19 06:12 joshuabezaleel

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. 🙇

mislav avatar Dec 04 '19 10:12 mislav

@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

mislav avatar Dec 04 '19 10:12 mislav

I have pushed some changes, mainly to:

  • compose GraphQL requests directly without the graphb library - this utilizes the Client.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.

mislav avatar Dec 16 '19 00:12 mislav

Will be this continued, since now the most attention will go to the CLI instead?

XVilka avatar Feb 17 '20 04:02 XVilka

Are there any updates regarding this PR? It needs to be rebased.

XVilka avatar Apr 14 '20 08:04 XVilka

@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.

mislav avatar Apr 14 '20 09:04 mislav

I would really love this feature. Is anyone working on this still?

dawsbot avatar Aug 12 '20 17:08 dawsbot

@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! 🙇

mislav avatar Aug 13 '20 11:08 mislav