RFR: 1997: Allow the creator of a PR to set the commit author [v3]

Erik Joelsson erikj at openjdk.org
Fri Sep 1 21:33:02 UTC 2023


On Fri, 1 Sep 2023 16:25:03 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> 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

Was there a reason you didn't add the comments as a field in `CheckablePullRequest`? I think that would make more sense as the `PullRequest` already is a field.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1553#discussion_r1313546162


More information about the skara-dev mailing list