RFR: 2509: Don't display duplicate jcheck warnings when the checks are displayed in Progress [v2]

[Erik Joelsson]

On Thu, 29 May 2025 16:51:39 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 422:
>> 
>>> 420:                 for (var annotation : visitor.getAnnotations()) {
>>> 421:                     checkBuilder.annotation(annotation);
>>> 422:                 }
>> 
>> I don't think we can assume that all `annotations` are whitespace warnings in this method. I don't think we need a summary text here, but it's hard to tell from just reading the code. The "Optional" title instead of "Required" and the list of annotations should be enough I think.
>
> Yes, for now, only whitespace check would generate annotations, but later, we may have other checks can generate annotations. But without a summary, GitHubPullRequest won't add the annotations to check. see: https://github.com/openjdk/skara/blob/131e80e440eacb6434544ec9d93f385601cce996/forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java#L480
> 
> So we need to put something else in the summary.

I was trying to understand the role of the summary and it seems when there are errors, we are listing the 50 first errors in the summary. In GitHub, when creating a `check`, we are then adding the summary and each annotation as an `annotation`, so this makes sense. In GitLab, there is no concept of a `check` so we print in a comment, first the summary and then the annotations. This must result in duplicate listings for the 50 first whitespace errors in GitLab. That's what had me confused and why I thought the summary wasn't necessary.

So in this case, as you are pointing out, we need to put something there. How about:


"These warnings will not block integration"

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

PR Review Comment: https://git.openjdk.org/skara/pull/1719#discussion_r2114737560