RFR: 1806: Optimize the CSR progress message and log when CSR not found

Erik Joelsson erikj at openjdk.org
Thu Jan 26 21:45:42 UTC 2023


On Thu, 26 Jan 2023 21:12:17 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> 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.
>
>> 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.
> 
> Good idea, I will split them into different methods. Thx!

I started looking at this call chain some more. We are already fetching issues multiple times, that's really what's causing this problem in the first place. We should only fetch issues once in a CheckRun. It will take some refactoring, since we need to preserve the IssueTracker Issues for CSR issues, but still worth it I think.

So instead of sending the fixVersion around, we should be fetching issues at a higher level and send those around.

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

PR: https://git.openjdk.org/skara/pull/1463


More information about the skara-dev mailing list