RFR: 2198: Backport PRs should check if a CSR is required [v6]

Erik Joelsson erikj at openjdk.org
Fri Mar 22 19:58:07 UTC 2024


On Fri, 22 Mar 2024 17:30:46 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> This patch is trying to let the pr bot automatically requires csr for a backport PR if one of the associated has a resolved CSR.
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add comment

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1648:

> 1646:      * Iterate through each entry in the regularIssuesMap. If the value is present, get all CSR links associated with this issue.
> 1647:      * "all CSR links" means that the CSR linked with the primary issue and the CSRs linked with any backport issues of the primary issue.
> 1648:      * Constructs a map from main issue ID to all CSR links.

I would like to comment to be more about what the return value is, not how it's created.
Suggestion:

     * Creates a map from issue ID to a list of all CSRs linked from the issue or any backport of the issue.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1650:

> 1648:      * Constructs a map from main issue ID to all CSR links.
> 1649:      */
> 1650:     private Map<String, List<Link>> issueToCsrLinksMap(Map<Issue, Optional<IssueTrackerIssue>> regularIssuesMap) {

Looking at this some more, I think it would make more sense if we made the map from issue ID to `List<IssueTrackerIssue>`. Both consumers of this map map to the linked Issue anyway.
Suggestion:

    private Map<String, List<IssueTrackerIssue>> issueToAllCsrsMap(Map<Issue, Optional<IssueTrackerIssue>> regularIssuesMap) {

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

PR Review Comment: https://git.openjdk.org/skara/pull/1623#discussion_r1536121261
PR Review Comment: https://git.openjdk.org/skara/pull/1623#discussion_r1536127517


More information about the skara-dev mailing list