RFR: 1691: Run Jcheck twice in CheckRun if .jcheck/conf has changed

Zhao Song zsong at openjdk.org
Wed Dec 7 22:33:34 UTC 2022

On Wed, 7 Dec 2022 21:19:35 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> When a pr is created, pr bot will run jcheck to check whether this commit is good. However, currently, we only run jcheck with the configuration from the target branch. Sometimes, user wants to modify jcheck configuration and accidentally messes up the format. In this case, pr bot would not be able to find this problem. After this commit with badly formatted jcheck configuration integrated, all the jcheck will fail and our bot gets stuck into a retry loop.
>> In this patch, if jcheck configuration file is updated, the bot will run jcheck twice, once with the configuration in the target and once with the new one in the source. So if the new configuration has any problem, it will became an integration blocker.
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckablePullRequest.java line 209:
>> 207: 
>> 208:     void executeChecks(Hash localHash, CensusInstance censusInstance, PullRequestCheckIssueVisitor visitor,
>> 209:                        List<String> additionalConfiguration, boolean useTargetConf) throws IOException {
> Instead of a boolean `useTargetConf`, I think it's better if we just supply the hash we want to use (target or src, if confOverride is set, then this method will handle it).

Yeah, pass hash is better. But even if we pass hash here, it won't affect the logic of using confOverride because confOverride is set in the constructor of `CheckablePullRequest`


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

More information about the skara-dev mailing list