RFR: 1937: Re-evaulate all PRs when .jcheck/conf changes [v2]

Erik Joelsson erikj at openjdk.org
Wed Jun 14 14:23:04 UTC 2023

On Tue, 13 Jun 2023 22:56:02 GMT, Zhao Song <zsong at openjdk.org> wrote:

>> As Erik said in the issue: "When we bump the version in .jcheck/conf from N to N+1 in the jdk master branch, PRs with CSRs are often not updated to reflect this."
>> This root cause of this is that changing of .jcheck/conf in target ref of the pr will not be able to trigger checkWorkItem for this pr.
>> In this patch,
>> 1. The prBot will monitor the changes to the .jcheck/conf file in any branch(except for PR branches).
>> 2. If the prBot detects an update to the .jcheck/conf file in a branch, it will trigger a checkWorkItem for all open PRs whose target ref is the branch where the .jcheck/conf file was updated.
>> 3. To solve the problem related with bot restarting, the contents of the .jcheck/conf file in the target branch of a PR will be included in the metadata hash.
> Zhao Song has updated the pull request incrementally with one additional commit since the last revision:
>   update

forge/src/main/java/org/openjdk/skara/forge/HostedRepository.java line 226:

> 224:      * Returns a list of open pull requests which targets at the specific ref
> 225:      */
> 226:     List<PullRequest> openPullRequestsTargetsSpecificRef(String targetRef);


    List<PullRequest> openPullRequestsWithTargetRef(String targetRef);

jcheck/src/main/java/org/openjdk/skara/jcheck/JCheckConfiguration.java line 158:

> 156:     public static JCheckConfiguration parse(List<String> lines) {
> 157:         var ini = INI.parse(lines);
> 158:         var rawConf = String.join("", lines);

This isn't actually the raw conf anymore, since it's been split by lines and then put together, but the original line breaks are lost. If we are calling this the "rawConf" then we should at least put `\n` back in there. Alternatively, we could store just a hash of the lines to make it clear that it can't be used to compare to another raw source of .jcheck/conf in the future.

I think just putting linebreaks in is enough for now.


PR Review Comment: https://git.openjdk.org/skara/pull/1531#discussion_r1229671897
PR Review Comment: https://git.openjdk.org/skara/pull/1531#discussion_r1229700208

More information about the skara-dev mailing list