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