Skip to content

ci(conventions): check pull requests with the current base branch rules - #8892

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
harisibrar1:ci/conventions-current-base
Oct 21, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
harisibrar1:ci/conventions-current-base

Conversation

@harisibrar1

Copy link
Copy Markdown
Contributor

What

The Conventions workflow checked out pull_request.base.sha, the base recorded when the pull request was opened, and ran that revision's checker. A rule merged after a pull request was opened never ran on it. #7005 passed with the checker of 07e7414, which had no env-reference rule, and merged APS5_NO_WGI without an ENVIRONMENT.md row.

  • conventions.yml: check out the current tip of pull_request.base.ref and pass --base HEAD. Check already diffs from git merge-base, so only the pull request's own changes are checked. It still runs only base-branch code and never executes pull request code.
  • check_conventions.py: an older branch lacks ENVIRONMENT.md rows added on the base branch after its branch point. Such a row now counts as documented unless the pull request removes it. Without this, feat(input): add native SDL input configuration with ImGui #674 is reported for ANYPS5_INPUT_CONFIG, which main already documents.
  • CONTRIBUTING.md: say which checker revision runs.

Effect: an open pull request that breaks a rule merged after it was opened fails on its next push, edit or reopen.

Tested

Windows 11, Python 3.14.

Checklist

🤖 Generated with Claude Code

The workflow checked out pull_request.base.sha, the base recorded when the
pull request was opened, and ran that revision's check_conventions.py. A rule
merged after a pull request was opened never ran on it: boykopovar#7005 added
APS5_NO_WGI without an ENVIRONMENT.md row and passed with the checker of
07e7414, which had no env-reference rule.

Check out the current tip of the base branch and pass it as --base; the
checker already diffs from the merge base, so only the pull request's own
changes are checked. A name documented on the base branch after the branch
point now counts as documented unless the pull request removes its row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix error Closes an issue (Fixes #N) label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 6044 CONTRIBUTING.md
PR 9137 tools/check_conventions.py

@harisibrar1

Copy link
Copy Markdown
Contributor Author

Checked by hand: #6044 adds a section at CONTRIBUTING.md line 21 (this PR changes line 44), and #8777 changes code() and symbol_nid in tools/check_conventions.py (this PR changes Check.__init__ and env_reference). No shared lines.

@MarcZX10 MarcZX10 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.

I reviewed the base-tip checkout and checker changes, including the merge-base diff behavior and the ENVIRONMENT.md rows inherited from the current target branch. I ran python3 -m unittest discover -s tools/tests -p "test_check_conventions_*.py" (143 passed) and python3 tools/check_conventions.py --base upstream/main (passed) on this head. The PR checks also pass. I found no blocking issue; the workflow integration itself still awaits its first GitHub run after merge.

@adriadam10 adriadam10 self-assigned this Oct 21, 2026
@adriadam10
adriadam10 merged commit 1896871 into boykopovar:main Oct 21, 2026
10 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the workflow change (current base tip with --base HEAD, merge-base still scopes the diff, only base code runs), the ENVIRONMENT.md rows inherited from the target (minus rows the PR drops) and the three new tests; the inherited-row test needs the check_conventions.py change. Conventions, intake and labels are green on the head; merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci .github docs fix error Closes an issue (Fixes #N)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci(conventions): pull requests are checked with the rules of the base they were opened on, so rules merged later never run on them

3 participants