RFR: 1393: PR bot fails on missing .jcheck/conf

Erik Joelsson erikj at openjdk.org
Tue Nov 1 17:35:43 UTC 2022


On Tue, 1 Nov 2022 17:15:35 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 231:
>> 
>>> 229:             //check whether this repo contains .jcheck/conf, if not, let the pr author to notify the repo owner
>>> 230:             try {
>>> 231:                 pr.repository().fileContents(".jcheck/conf", pr.targetRef());
>> 
>> Instead of explicitly checking for `.jcheck/conf` here through a remote call, I think it would be better to react to if the `Optional<CensusInstance>` below comes back empty or not. If it's empty and `bot.confOverrideRepository().isEmpty()` then you can print a message.
>
>> Instead of explicitly checking for `.jcheck/conf` here through a remote call, I think it would be better to react to if the `Optional<CensusInstance>` below comes back empty or not. If it's empty and `bot.confOverrideRepository().isEmpty()` then you can print a message.
> 
> Yesterday I was wondering whether there is any possible that `Optional<CensusInstance>` is empty due to other errors. Today I read through the code again and confirm that Only `.jcheck/conf` invalid or missing will make `Optional<CensusInstance>` empty. Thx!

Maybe it would be even better to throw an exception instead of returning Optional.empty(). We can't recover from it  in CheckWorkItem anyway, and with a typed exception we can communicate more exactly what went wrong, and we can catch exactly that exception type and write the message.

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

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


More information about the skara-dev mailing list