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