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

[Erik Joelsson]

On Tue, 15 Nov 2022 22:57:38 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> SKARA-1393 describes a problem related with `.jcheck/conf` missing.
>> 
>> In this patch, when the problem happens, the author of this pull request will get instructions about how to solve this problem.
>
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Updated CommitCommandTests#missingJcheckConf

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 227:

> 225:         var seedPath = bot.seedStorage().orElse(scratchPath.resolve("seeds"));
> 226:         var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
> 227:         CensusInstance census = null;

I don't think you need to assign null here as all exception paths will return.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 241:

> 239:                 addErrorComment(text, comments);
> 240:             } else {
> 241:                 log.info("No .jcheck/conf found in external repo " + bot.confOverrideRepository().get().name() + ": " + e);

The file isn't necessarily called `.jcheck/conf`, the name is given in `bot.confOverrideName()`.

The log should be at SEVERE level and the throwable should be included. To achieve this, you need to use the `log` method. By making it SEVERE, we will get admin notifications about this.

Suggestion:

                log.log(Level.SEVERE, "Jcheck configuration file " + bot.confOverrideName() + " not found in external repo " + bot.confOverrideRepository().get().name(), e);

bots/pr/src/main/java/org/openjdk/skara/bots/pr/LimitedCensusInstance.java line 77:

> 75:             conf = Optional.of(Arrays.stream(remoteRepo.fileContents(name, ref).split("\n")).toList());
> 76:         } catch (NoSuchElementException ignored) {
> 77:             // NoSuchElementException will only work for tests

I'm not sure I understand this part.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCommandWorkItem.java line 199:

> 197:         var hostedRepositoryPool = new HostedRepositoryPool(seedPath);
> 198: 
> 199:         CensusInstance census = null;

I don't think you need to assign null here.

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

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