dlang-bot icon indicating copy to clipboard operation
dlang-bot copied to clipboard

Query CI providers for PR number

Open wilzbach opened this issue 8 years ago • 5 comments

Problem: in the GH status hooks there's no reference of the PR number and the API also doesn't provide a way to match the commit sha to a respective PR. However most CI providers maintain a reference on their site, so we can simply query them. This main part of this PR implements this.

Moreover it adds the following check to the GH Hook handler as a proof-of-concept to show that fetching the PR number works. Future work can extend these checks arbitrarily:

IF status == "success" -> query GH API for all current CI statuses IF two or more are "success" -> figure our PR number (abort on not found) -> query GH review API IF no reviews found -> add "needs review" label IF review found -> remove "needs review"

Other

  • added addAPIExpections
  • fixed bug in payloadServer (somehow writing a Json response with toString triggered undefined behavior)
  • made the PRReview check optional in the test suite (it doesn't make sense to update all tests with their exact API expectations all the time)

wilzbach avatar Feb 20 '17 02:02 wilzbach

Another benefit of this is that we can detect failures by a CI and connect them to a PR, which should be quite interesting as the auto-tester tries to rebuild PRs constantly and thus will let us know if the PR isn't buildable, mergeable etc.

wilzbach avatar Feb 22 '17 05:02 wilzbach

One question here: is it all possible to be more precise about what CI are allowed to pass?

I would suggest that some CI are more important than others -- so e.g. style CI should not block moving to 'needs review', but test-suite failure should probably see the tag set to 'needs work' (maybe with a friendly message about how to address the test-suite failure the first time it happens).

WebDrake avatar Feb 22 '17 21:02 WebDrake

I would suggest that some CI are more important than others

Hehe at the moment GH is still in the "all CIs are equal stage", maybe soon we will have "all CIs are equal, but some are more equal" ;-)

One question here: is it all possible to be more precise about what CI are allowed to pass?

AFAIK this is not exposed via the GH API and I can think of these three options:

  • hardcode what CIs are required/"more important"
  • simulate login as user and scrape the PR page with the received cookie (I have already implemented sth. similar here: https://github.com/dlang-bots/dlang-bot/pull/50)
  • have a simple plain file within the repo that provides this knowledge (there's no API rate limit on raw files!)

So yeah I absolutely agree with you about this, but I wanted to keep this already large PR as small as this is really just intended to be able to know for what PR we just received an event which is the first requirement to do anything like this.

(I have opened #58 for this)

wilzbach avatar Feb 22 '17 22:02 wilzbach

Quite unfriendly of GH to not do the translation, guess it's b/c a single commit status could apply to multiple PRs. In the longer run keeping track of sha1 -> PR relations ourselves would allow to avoid maintaining that integration.

MartinNowak avatar Mar 30 '17 17:03 MartinNowak

I think at this point it would be simpler to store locally which commit SHA1s belong to which PRs (when a PR is created or synchronized).

CyberShadow avatar May 27 '21 00:05 CyberShadow