Based on the discussion here: https://mail.openjdk.org/pipermail/jdk-updates-dev/2022-August/016451.html, I think we have agreed to keeping the approval process in JBS, but make Skara aware of it for a smoother user experience. When this enhancement was originally filed, there were limitations in the Skara bot architecture that made it expensive to poll bugs for updating the state of a PR. These limitations have now been solved, and having PR checks react to bug updates is close to trivial (since
In addition to the OpenJDK process for maintainer approval, there are some other similar processes that use labels in JBS to allow or block integration, each with different sets of labels and naming schemes. Because of this, we need the configuration to be rather flexible. I propose a format like this (examples):
The simple case, where the labels are the same for every branch in a repository:
"approval": {
"request": "jdk17u-fix-request",
"approved": "jdk17u-fix-yes",
"rejected": "jdk17u-fix-no",
}
To reduce the need for changing multiple strings when copying a configuration for a new repository, there is an optional "prefix" field:
"approval": {
"prefix": "jdk17u-fix-",
"request": "request",
"approved": "yes",
"rejected": "no",
}
When there are multiple branches with different labels, having the prefix set per branch can help reduce the size of the configuration significantly:
"approval": {
"request": "-critical-request",
"approved": "-critical-approved",
"rejected": "-critical-rejected",
"branches": [
"jdk20\\.0\\.1": { "prefix": "CPU23_04" }
]
}
So, each of "request", "approved" and "rejected" are the base label names for the three different functions. There is an optional prefix that gets applied to all labels, and this prefix can be overridden for each branch, or for a regex expression matching a branch.
When the "approval" configuration contains a "branches" field, then only branches that match any of those regexp elements will be subject to the approval requirement.
In CheckRun, when listing the associated bugs, we should add the current approval state to each bug like this to make the current state easy to see:
JDK-000000: Foo bar bug0 (Bug - P4)
JDK-000001: Foo bar bug1 (Bug - P4 - Requested)
JDK-000002: Foo bar bug2 (Bug - P4 - Approved)
JDK-000003: Foo bar bug3 (Bug - P4 - Rejected)
In botSpecificProgresses, we need to check the approval state of all associated issues and provide the checkbox line, simliar to CSR and JEP.
When evaluating the PR metadata hash to see if CheckWorkItem needs to perform a new check, we need to add the set of labels from all associated issues.
When a PR has been reviewed, but is still missing maintainer approval on bugs, we should add a comment in place of the "mergeReadyComment" that explains the need for maintainer approval and links to the process. This link should probably be configurable as different repos may have different processes. Something like: "This pull request has now been reviewed. Before it can be integrated, it requires additional approval." (where "additional approval" is a link)
- blocks
-
SKARA-1949 Handle maintainer approval from pull request
- Resolved
- relates to
-
SKARA-2179 Configure jfx17u for maintainer approval
- Resolved
-
SKARA-2041 Add configuration for maintainer approval to jdk11u-dev
- Resolved
-
SKARA-2074 Add configuration for maintainer approval to jdk11u
- Resolved
-
SKARA-2075 Add configuration for maintainer approval to jdk17u
- Resolved
-
SKARA-1433 The change of the CSR status doesn't force the CheckWorkItem to re-run the check
- Resolved
-
SKARA-1575 Adjust the pull request APIs in HostedRepository
- Resolved
-
SKARA-1576 Refactor 'CSRIssueBot' and 'CSRPullRequestBot' by using the newly added poller class
- Closed