RFR: 1806: Optimize the CSR progress message and log when CSR not found
Erik Joelsson
erikj at openjdk.org
Thu Jan 26 20:53:02 UTC 2023
On Thu, 26 Jan 2023 18:19:15 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> I add this field because I don't want to call `BotUtils.getVersion(pr)` twice. If adding a field is a bad idea, I think just call `BotUtils.getVersion(pr)` twice is better than add `JdkVersion` to many method interfaces.
>
> I think the calling tree of `issues()` is in a mess because we have two different type of issues(`org.openjdk.skara.vcs.openjdk` and `org.openjdk.skara.issuetracker.Issue`). Besides, we don't have class fields to store issues, csrIssues, JepIssues. So we need to call `issues()` three times.
Yes, the two different Issue types definitely adds to the confusion, but we also have the `::issues` method that takes two booleans. It has three callers and only one of those sets the parameters true. This should just have been split into different methods. As it is now, they are calling each other in a circle, only saved by the boolean being false in the second call. Untangling this would make it pretty straightforward to provide a JdkVersion argument to the method for getting just CSR issues.
I think there is value in only calling `BotUtils.getVersion(pr)` once as it's quite expensive to evaluate.
-------------
PR: https://git.openjdk.org/skara/pull/1463
More information about the skara-dev
mailing list