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

[Erik Joelsson]

On Mon, 25 Apr 2022 23:30:42 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> I tried this way at first when I solved this issue. But then I found the ReviewersCheck doesn’t always return the TooFewReviewersIssue which means the message may miss sometimes. And if we return the message in all the issues of the ReviewersCheck, the message may also miss when the ReviewersCheck passes without any issue.

Right, this would only work when the checkbox isn't checked.

My biggest issue here is that you are storing mutable state in `CheckablePullRequest`. That class is currently immutable.

Here is what I would suggest instead. Still move the logic for generating the reviewers description string to ReviewersConfiguration. Add a field to `PullRequestCheckIssueVisitor` for holding a Jcheck configuration. Set this field from `CheckablePullRequest::executeChecks`. The visitor is already a class that carries a mutable state, so this is kind of ok. Then in `PullRequestCheckIssueVisitor::get*Checks` explicitly modify the description for ReviewersCheck in the returned map. I would recommend a new private method that wraps the `Check::description` call and extracts the reviewer information from the conf field.

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

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