RFR: 2231: JCheck ran twice unnecessarily

[Erik Joelsson]

On Thu, 11 Apr 2024 20:19:42 GMT, Zhao Song <zsong at openjdk.org> wrote:

> Recently, I found sometimes JCheck ran twice unnecessarily in some prs.
> For example:
> https://github.com/openjdk/jdk/pull/16005
> https://github.com/openjdk/jdk/pull/14586
> 
> In the above prs, the authors didn't update .jcheck/conf in the source branch, but skara executed jcheck twice with the jcheck config in the merged commit(merged target branch and source branch).
> 
> After investigation,l  found that in this case, skara bot will make a mistake.
> Let's say we have two branches:
> Target branch: initialCommit <-- updateJCheckCommit <-- latestCommit
> Source Branch: initialCommit <-- editCommit
> In CheckRun, Skara bot would create a squashed commit(final head is editCommit and parent is latestCommit), and the squashed commit would include diffs in updateJCheckCommit, latestCommit, editCommit. Later, Skara bot would use CheckRun#isFileUpdated to check if .jcheck/conf is updated in the squashed commit and it will return true, therefore  run jcheck twice.
> 
> To resolve this problem, I think we should rewrite isFileUpdated, skara bot should find the common ancestor of target branch and source branch first, then check if the file is updated between mergeBase and the head of source branch.

I think this makes sense. Good find.

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

Marked as reviewed by erikj (Lead).

PR Review: https://git.openjdk.org/skara/pull/1635#pullrequestreview-1995775112