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

Erik Joelsson erikj at openjdk.org
Thu Jan 26 23:51:29 UTC 2023


On Thu, 26 Jan 2023 22:40:27 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 1131:
>> 
>>> 1129:                 version = BotUtils.getVersion(pr).orElse(null);
>>> 1130:             } catch (Exception e) {
>>> 1131:             }
>> 
>> I'm not sure ignoring an exception here is a good idea. In what situation did it throw one?
>
>> I'm not sure ignoring an exception here is a good idea. In what situation did it throw one?
> 
> It's because `BotUtils.getVersion` only works for fixVersion is configured in` .jcheck/conf`. If we use the override jcheck conf, it will throw exception. So it will make some override jcheck conf tests fail. I think we could fix it, but not in this pr.

It needs to be solved in some way. Catching `Exception` and ignoring it is not allowed in bot code. That could be triggered by an IOException, or failure in contacting the remote or almost anything else. If that happens, we need to handle the failure correct so that the WorkItem will be retried.

BotUtils.getVersion either can't throw RuntimeException when it can't find a .jcheck/conf file, or we need to handle the fallback to the override repo. The latter would be cleaner, but we probably don't really need that support. I think returning Optional.empty() from BotUtils.getVersion() is probably ok if no .jcheck/conf file could be found.

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

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


More information about the skara-dev mailing list