GHRepository.createCheckRun

[jglick]

Description

Continues #723 & #740 to create a check run. Partially addresses #520.

I am leery of building this on top of #724, which I can barely understand but anyway does not feel appropriate here since for example a GHCheckRun is not a complete model object: it has an Output nested object with an annotationsCount & annotationsUrl but there is not currently any separate model for the actual annotation data. Some sort of builder pattern is needed, but I find the style used here (inspired by GHPullRequestReviewBuilder) more intuitive and straightforward and it is clear what Java method actually corresponds to the REST call.

Before submitting a PR:

• Push your changes to a branch other than master. Create your PR from that branch.
• Add JavaDocs and other comments
• Write tests that run and pass in CI. See CONTRIBUTING.md for details on how to capture snapshot data.
• Run mvn -D enable-ci clean install site locally. This may reformat your code, commit those changes. If this command doesn't succeed, your change will not pass CI.

[XiongKezhi]

Do we also need another updateCheckRun method for this here? They are almost the same except for the URL and request method.

[jglick]

Do we also need another updateCheckRun method

Yes, could be added as well. AFAICT it is not a top priority since “creating” a run with the same name as an existing one effectively overwrites it.

[bitwiseman]

Add links to the pertinent GitHub docs, such as https://developer.github.com/v3/checks/runs/#actions-object.

[bitwiseman]

There's a maximum of three actions allowed. But the GitHub docs do not make it clear what happens when actions are exceeded. We should at least mention the limit here.

[jglick]

As elsewhere, I think it is pointless to even mention these sorts of limitations in Javadoc here, as the limits could be changed at any time and the code in this repository would become stale and misleading. (For example, some fields accept quite arbitrary-seeming lengths such as 20 characters.) It is the caller’s responsibility to follow GitHub’s current limits, and deal with whatever errors result if those are exceeded.

[jglick]

(The exception being the 50 annotation limit, since that has a documented and tested solution—adding annotations in batches of 50 or less—which can be implemented in the library.)

[bitwiseman]

Coming along really well.

[jglick]

I suppose unrelated flake:

java.lang.AssertionError: 
if header instance expires but queried instance is valid, ratelimit() uses it without asking server
Expected: not sameInstance(<GHRateLimit {core {remaining=4897, limit=5000, resetDate=Fri Mar 27 18:17:03 GMT 2020}search {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}graphql {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}integrationManifest {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}}>)
     but: was <GHRateLimit {core {remaining=4897, limit=5000, resetDate=Fri Mar 27 18:17:03 GMT 2020}search {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}graphql {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}integrationManifest {remaining=999999, limit=1000000, resetDate=Fri Mar 27 19:17:00 GMT 2020}}>
	at org.kohsuke.github.AbstractGitHubWireMockTest.assertThat(AbstractGitHubWireMockTest.java:261)
	at org.kohsuke.github.GHRateLimitTest.executeExpirationTest(GHRateLimitTest.java:417)
	at org.kohsuke.github.GHRateLimitTest.testGitHubRateLimitExpirationServerFiveMinutesAhead(GHRateLimitTest.java:343)

[bitwiseman]

We're discussing the design of parts of the API.

[jglick]

Can also look into regenerating all WireMock data files to use the new filename convention.

[jglick]

BTW you need to revisit tests from #595, which have flaked twice in this PR’s builds. Deleting Thread.sleep calls causes test failures, which is a “bad code smell”.

[bitwiseman]

Thanks, Jesse!