RFR: 1706: Store github checks in PR unique way
Zhao Song
zsong at openjdk.org
Thu Feb 9 22:26:54 UTC 2023
On Thu, 9 Feb 2023 22:01:37 GMT, Erik Joelsson <erikj 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/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.
But in that way, the checkName will be "jcheck-repo#pull" and the error message showed to user would be "the status check jcheck-repo#pull is still in progress".
> 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.
As you said in the comment above "The check is created in CheckRun, so the name needs to be changed there too.", I didn't change the check name when it is created, so I changed it here. It's my bad, I will fix it.
-------------
PR: https://git.openjdk.org/skara/pull/1470
More information about the skara-dev
mailing list