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

Erik Joelsson erikj at openjdk.org
Tue Nov 15 19:21:29 UTC 2022


On Fri, 4 Nov 2022 17:02:06 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:
> 
>   Used UncheckedRestException

This is starting to look pretty good.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CensusInstance.java line 58:

> 56:     }
> 57: 
> 58:     static Optional<CensusInstance> createCensusInstance(HostedRepositoryPool hostedRepositoryPool,

No need to return `Optional` here anymore either.

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

> 234:         } catch (MissingJCheckConfException e) {
> 235:             if (bot.confOverrideRepository().isEmpty()) {
> 236:                 var text = " ⚠️ @" + pr.author().username() + " The `.jcheck/conf` in the target branch of this pull request is missing completely. "

Suggestion:

                var text = " ⚠️ @" + pr.author().username() + " No `.jcheck/conf` found in the target branch of this pull request. "

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

> 238:                 addErrorComment(text, comments);
> 239:             } else {
> 240:                 var text = " ⚠️ @" + pr.author().username() + " The external jcheck configuration for this repository could not be resolved. "

Suggestion:

                var text = " ⚠️ @" + pr.author().username() + " The external jcheck configuration for this repository could not be found. "


The double "resolved" doesn't read well.

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

> 249:                 addErrorComment(text, comments);
> 250:             } else {
> 251:                 var text = " ⚠️ @" + pr.author().username() + " The external jcheck configuration for this repository could not be resolved. "

Suggestion:

                var text = " ⚠️ @" + pr.author().username() + " The external jcheck configuration for this repository is invalid. "

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CommitCommandWorkItem.java line 133:

> 131:             log.info("No new commit comments found, stopping further processing");
> 132:         } else {
> 133:             Optional<LimitedCensusInstance> census = Optional.empty();

No need to keep this as an `Optional` either.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CommitCommandWorkItem.java line 144:

> 142:                 var comment = String.format(commandReplyMarker, command.id()) + "\n" +
> 143:                         "@" + command.user().username() +
> 144:                         " there is no `.jcheck/conf` present at revision " +

Suggestion:

                        " There is no `.jcheck/conf` present at revision " +

bots/pr/src/main/java/org/openjdk/skara/bots/pr/CommitCommandWorkItem.java line 151:

> 149:                 var comment = String.format(commandReplyMarker, command.id()) + "\n" +
> 150:                         "@" + command.user().username() +
> 151:                         " invalid `.jcheck/conf` present at revision " +

Suggestion:

                        " Invalid `.jcheck/conf` present at revision " +

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

> 55:             var census = Census.parse(repoFolder);
> 56:             var namespace = namespace(census, repository.namespace());
> 57:             return Optional.of(new LimitedCensusInstance(census, configuration, namespace));

No need to return `Optional` here anymore as we throw exception when the configuration is missing.

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

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


More information about the skara-dev mailing list