RFR: 2479: PR marked as ready with jcheck error [v2]
Zhao Song
zsong at openjdk.org
Thu Apr 17 20:34:54 UTC 2025
On Thu, 17 Apr 2025 20:24:19 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> 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)
Yes, sounds good
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1714#discussion_r2049612066
More information about the skara-dev
mailing list