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