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