Skip to content

.github/workflows/check.yml: compute the checkpatch range from the merge base - #20696

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
raiden00pl:check-merge-base-pr
Oct 16, 2026
Merged

xiaoxiang781216 merged 1 commit into
apache:masterfrom
raiden00pl:check-merge-base-pr

Conversation

@raiden00pl

Copy link
Copy Markdown
Member

Summary

github.event.pull_request.base.sha is a cached value that GitHub updates asynchronously, so a synchronize event can carry the base SHA from when the PR was opened even after the branch was rebased onto the current base. The checked range then spans every commit that landed on the base branch since, and any commit there that fails checkpatch fails the PR.

Compute the range from the merge base against the live base branch so only the PR commits are checked.

Impact

try to fix broken CI: https://github.com/apache/nuttx/actions/runs/38791720234/job/116617691575#step:3:253

Testing

none

…rge base

github.event.pull_request.base.sha is a cached value that GitHub updates
asynchronously, so a synchronize event can carry the base SHA from when
the PR was opened even after the branch was rebased onto the current
base. The checked range then spans every commit that landed on the base
branch since, and any commit there that fails checkpatch fails the PR.

Compute the range from the merge base against the live base branch so
only the PR commits are checked.

Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
@github-actions github-actions Bot added Area: CI Size: XS The size of the change in this PR is very small labels Oct 15, 2026
@casaroli

Copy link
Copy Markdown
Contributor

yes, please

@casaroli

Copy link
Copy Markdown
Contributor

I tested this on #20640 stale merge commit and I can confirm this is a huge developer quality of life improvement.

@github-actions

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@raiden00pl should we change apps too?

@cederom cederom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @raiden00pl :-)

We are loosing format fix for other places that way :-P

@xiaoxiang781216
xiaoxiang781216 merged commit caa178d into apache:master Oct 16, 2026
49 of 53 checks passed
acassis pushed a commit to apache/nuttx-apps that referenced this pull request Oct 16, 2026
…rge base.

The check job checked github.event.pull_request.base.sha..HEAD. HEAD is
the merge commit that GitHub made for the pull request, and that merge
commit can be on an older base than base.sha. checkpatch then diffs the
two endpoints, so every change on master since that older base looks
like a change of the pull request. Old files in that range fail the
check.

Fetch the base branch and check from the merge base of the base branch
and HEAD. This is the same change as apache/nuttx#20696.

Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Marco Casaroli <marco.casaroli@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants