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

Zhao Song zsong at openjdk.org
Wed Dec 7 23:28:32 UTC 2022


On Wed, 7 Dec 2022 22:30:04 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> 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`

Now, I prefer not to change this method because I think ` CheckablePullRequest#targetHash` can help us reuse the target hash. If we add hash as an argument, we will need to call `PullRequestUtils.targetHash(localRepo)` more than once.

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

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


More information about the skara-dev mailing list