RFR: 1997: Allow the creator of a PR to set the commit author [v2]
Zhao Song
zsong at openjdk.org
Fri Sep 1 16:27:24 UTC 2023
On Fri, 1 Sep 2023 14:09:18 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> Zhao Song has updated the pull request incrementally with three additional commits since the last revision:
>>
>> - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>>
>> Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>> - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>>
>> Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>> - Update bots/pr/src/main/java/org/openjdk/skara/bots/pr/AuthorCommand.java
>>
>> Co-authored-by: Erik Joelsson <37597443+erikj79 at users.noreply.github.com>
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 181:
>
>> 179: }
>> 180:
>> 181: var authorSet = Authors.author(pr.repository().forge().currentUser(), pr.comments());
>
> Calling `pr.comments()` here is a remote call so rather expensive. It's already done in `commitMessage`. We could at least make sure we only call it once. Looking a bit further, I see that in all places where a `CheckablePullRequest` is created, we already have a list of comments, so I think an even better solution here is to add a `comments` field and supply it to the constructor.
>
> I think the variable name can be clearer.
> Suggestion:
>
> var overridingAuthor = Authors.author(pr.repository().forge().currentUser(), pr.comments());
Sure, will reuse the comments
-------------
PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313250522
More information about the skara-dev
mailing list