RFR: 1706: Store github checks in PR unique way [v2]

Erik Joelsson erikj at openjdk.org
Fri Feb 10 14:20:42 UTC 2023

On Fri, 10 Feb 2023 11:56:36 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
>>   fix problems
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1091:
>> 1089:         return pr.repository().forge().name().equals("GitHub") ? "jcheck-" + pr.repository().name() + "-" + pr.id() : "jcheck";
>> 1090:     }
>> 1091: 
> Why does the `GitLab merge request` not need to change such name? I think GitLab also needs.
> This method is now put in class `CheckRun` and is used in both `CheckRun` and `CheckWorkItem`. It seems not a good design. We can move the method to the interface `PullRequest` with a default implementation, just like below. Then we can use `pr.getJcheckName()` in both `CheckRun` and `CheckWorkItem`.
> // file PullRequest.java
>     default String getJcheckName() {
>         return "jcheck-" + repository().name() + "-" + id();
>     }

Because there is no concept of a "check" in GitLab. We just store the check results in a comment in the PR, so there is no reason to make the name of the check unique.

Until now my opinion was that the name of the jcheck check was something only the pull request bot module needed to know about, so putting that on the `PullRequest` interface didn't seem right. It was an implementation detail. But now since we need to share this information with testinfo, it would make sense to push this logic into `PullRequest`. The default method should in that case return "jcheck" and `GitHubPullRequest` can have the special implementation needed only on GitHub.


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

More information about the skara-dev mailing list