RFR: 1856: Include a link to the webrev comment in the pull request's description

Erik Joelsson erikj at openjdk.org
Thu Mar 30 18:48:38 UTC 2023


On Wed, 29 Mar 2023 21:58:06 GMT, Zhao Song <zsong at openjdk.org> wrote:

> Some users complained about that GitHub often hide the webrev comment and it's hard for them to find webrev comment among hundreds of comments. 
> 
> In this patch, the pr bot would add the link to webrev comment to the pr body.
> 
> Originally, the comments from mlbridge bot would not trigger the update of the pr, now, when mlbridge bot **first** post the webrev comment, it will trigger the update of the pr.

Overall a nice solution to this issue.

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

> 733:     private String getWebrevCommentLink() {
> 734:         var webrevComment = comments.stream()
> 735:                 .filter(comment -> comment.author().username().equals("mlbridge[bot]"))

We can't hard code the bot name here, it needs to be a configuration parameter for pr bot. See configuration of mlbridge for an example of how we configure user names to filter comments.

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

> 737:                 .findFirst();
> 738:         return webrevComment.map(comment -> "[Link to Webrev Comment](" + pr.commentUrl(comment).toString() + ")")
> 739:                 .orElse("Webrev comment has not been available now.");

I think this method could return an Optional and that we should only print the whole section if a webrev comment has been posted.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 103:

> 101:                     .flatMap(comment -> comment.body().lines())
> 102:                     .filter(line -> line.equals(WEBREV_COMMENT_MARKER))
> 103:                     .collect(Collectors.joining());

Since we only want to react when this comment is added the first time, we could do `findFirst` here. That way we terminate the stream once we find the relevant comment.

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

PR Review: https://git.openjdk.org/skara/pull/1494#pullrequestreview-1365727700
PR Review Comment: https://git.openjdk.org/skara/pull/1494#discussion_r1153650416
PR Review Comment: https://git.openjdk.org/skara/pull/1494#discussion_r1153651687
PR Review Comment: https://git.openjdk.org/skara/pull/1494#discussion_r1153656732


More information about the skara-dev mailing list