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