RFR: 2331: Improve review notes
Zhao Song
zsong at openjdk.org
Thu Jul 18 17:59:41 UTC 2024
On Thu, 18 Jul 2024 10:52:54 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
> There are two variants of a note that can be added to an item in the "Reviewers" list of a PR body:
>
> - 🔄 Re-review required ...
> - ⚠️ Review applies to ...
>
> That note may confuse or draw unneeded attention to the PR body.
>
> Firstly, as a result of recently integrated [SKARA-2312][], "⚠️ Review applies to ..." is also added to an item (i.e. review) that applies to the head of the PR. Secondly, "🔄 Re-review required (review applies to ..." may be present for some items even though a PR has the green "ready" label.
>
> [SKARA-2312]: https://bugs.openjdk.org/browse/SKARA-2312
bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1351:
> 1349: jcheckType = "target jcheck";
> 1350: var issues = checkablePullRequest.executeChecks(localHash, censusInstance, visitor, targetJCheckConf);
> 1351: tooFewReviewers = issues.stream().anyMatch(TooFewReviewersIssue.class::isInstance);
I am not sure if it's a good idea to change the return value of `CheckablePullRequest.executeChecks`.
I think you can use `visitor.getChecks()` to determine if reviewers check failed or not.
var tooFewReviewers = visitor.getChecks()
.entrySet().stream()
.anyMatch(entry -> entry.getKey().contains("Change must be properly reviewed") && entry.getValue().equals(false));
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1679#discussion_r1683270282
More information about the skara-dev
mailing list