RFR: 2479: PR marked as ready with jcheck error [v2]

Erik Joelsson erikj at openjdk.org
Thu Apr 17 20:26:26 UTC 2025


On Thu, 17 Apr 2025 20:08:01 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> Do you mean that `tooFewReviewers()` should return true when there is only ReviewersCheck Failure?
>
> Or does `tooFewReviewers()` return true when there is a ReviewersCheck Failure in the list?
> If so, the logic should be  like this
> `            var readyToPostApprovalNeededComment = readyForReview &&
>                     ((!reviewNeeded && visitor.errorFailedChecksMessages().size() == 1 && visitor.tooFewReviewers()) ||
>                             visitor.errorFailedChecksMessages().isEmpty()) &&
>                     integrationBlockers.isEmpty() &&
>                     !statusMessage.contains(TEMPORARY_ISSUE_FAILURE_MARKER);`

Right, I didn't think that through and this gets messy and I think it's because we aren't using the right abstraction. Can we just push something like this down to the visitor:

boolean hasErrors(boolean reviewNeeded)

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

PR Review Comment: https://git.openjdk.org/skara/pull/1714#discussion_r2049597512


More information about the skara-dev mailing list