RFR: 282: Make the review requirements more explicit in the progress list

Erik Joelsson erikj at openjdk.java.net
Mon Apr 25 23:20:25 UTC 2022


On Sun, 24 Apr 2022 15:59:42 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch adds the explicit review requirements, such as ` (2 reviews required, with at least 1 Reviewer).`, to the progress list and adds some test cases.
> 
> Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

I don't think this is the right solution. After having looked through the various classes involved here, I think I would put the logic for generating a string describing the reviewer requirements in the ReviewersConfiguration class. Then I would keep PullRequestCheckIssueVisitor responsible for extracting the proper message for each kind of issue returned from the get*Checks methods, instead of modifying them in CheckRun. To do this the TooFewReviewersIssue would need to hold the requirements string from the configuration, but I think it makes enough sense for it to do that. ReviewersCheck has access to it and can provide it to the constructor.

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

PR: https://git.openjdk.java.net/skara/pull/1305


More information about the skara-dev mailing list