RFR: 286: Add more info about CSR in the PR's first comment [v2]

Erik Joelsson erikj at openjdk.java.net
Wed Nov 24 14:00:06 UTC 2021

On Wed, 24 Nov 2021 07:27:06 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Hi all,
>> This patch adds a link to the csr in the PR's first comment under **Issue**.
>> And the corresponding test is added.
>> Thanks for taking the time to review.
>> Best Regards,
>> -- Guoxiong
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>   Fix null exception.

Overall I like this change. I'm wondering if we should add something like "CSR" either before or after the CSR link to make it clearer what the issue link is. I would suggest a bold CSR within normal parentheses after the title. This matches the formatting of the reviewer attributions further down in the body.

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

> 135:                 if (relationship.isPresent() && relationship.get().equals("csr for")) {
> 136:                     csr = link.issue().orElseThrow(
> 137:                             () -> new IllegalStateException("Link with title 'csr for' does not contain issue")

I don't think we should throw an exception here. If we do, then a bad state in JBS will completely prevent the PullRequestBot from updating the status of the affected PR. I would prefer logging a warning and returning Optional.empty().


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

More information about the skara-dev mailing list