RFR: 1706: Store github checks in PR unique way

[Erik Joelsson]

On Thu, 9 Feb 2023 19:13:37 GMT, Zhao Song <zsong at openjdk.org> wrote:

> Currently, checks in GitHub are stored in the commit. So if multiple PRs share the same source commit, then they will compete with the check-runs and store their different checksum in there. This in turn means that all, or most of those PRs will always think the check is outdated when a CheckRun is performed next.
>  
> This patch adds repository name and pull request id to the name of check in Github. Therefore, even if multiple PRs share the same source commit, every PR will store a unique check in the commit and there is no competition exists.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 123:

> 121:         var metadata = getMetadata(censusInstance, pr.title(), pr.body(), comments, reviews, labels, pr.targetRef(), pr.isDraft(), null);
> 122:         var currentChecks = pr.checks(hash);
> 123:         var jcheckName = pr.repository().forge().name().equals("GitHub") ? "jcheck" + pr.repository().name() + pr.id() : "jcheck";

The generation of the name needs to be handled in one place. Possibly a static method on CheckRun or CheckWorkItem, or maybe CheckablePullrequest. I would also suggest some delimiters, something like `jcheck-repo#pull` (if # is a valid character, otherwise just dash).

The check is created in CheckRun, so the name needs to be changed there too.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/IntegrateCommand.java line 64:

> 62:         final var outdated = "the status check `" + checkName + "` has not been performed on commit %s yet";
> 63: 
> 64:         checkName = checkName.equals("jcheck") && pr.repository().forge().name().equals("GitHub") ?

There is just one caller of this method, which explicitly calls with `"jcheck"`. The caller should generate the new check name instead of rewriting it here.

forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java line 464:

> 462:     public void updateCheck(Check check) {
> 463:         var completedQuery = JSON.object();
> 464:         completedQuery.put("name", check.name().equals("jcheck") ? check.name() + repository().name() + id() : check.name());

I don't understand this change. Why would we need that? I don't think GitHubPullRequest should be changing check names.

-------------

PR: https://git.openjdk.org/skara/pull/1470