RFR: 1683: The force push notifier needs an overhaul [v2]

Erik Joelsson erikj at openjdk.org
Wed Nov 23 18:47:28 UTC 2022


On Wed, 23 Nov 2022 00:00:29 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 392:
>> 
>>> 390:             }
>>> 391: 
>>> 392:             // Check Force push
>> 
>> I'm not sure if we should put this check here in `CheckWorkItem`. This will cause it to be run quite often. Maybe it would be better to put it in `CheckRun` where it would be guarded by the check metadata condition, so it would only need to be run if something significant has changed since the last check.
>
>> I'm not sure if we should put this check here in `CheckWorkItem`. This will cause it to be run quite often. Maybe it would be better to put it in `CheckRun` where it would be guarded by the check metadata condition, so it would only need to be run if something significant has changed since the last check.
> 
> I think this is already guarded by the check metadata condition. It will only be executed when `currentCheckValid(census, comments, activeReviews, labels)` is false. The if clause is very long(line 273-425).

You are right, by bad. I misremembered the location of currentCheckValid. This looks like the right place to do this then.

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

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


More information about the skara-dev mailing list