Skip to content

fix(ci): make version-bump check against merge-base - #330

Merged
jplbrun merged 1 commit into
mainfrom
ci/version-bump-merge-base
Sep 14, 2026
Merged

jplbrun merged 1 commit into
mainfrom
ci/version-bump-merge-base

Conversation

@jplbrun

@jplbrun jplbrun commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

The Check Version Bump workflow diffs github.event.pull_request.base.sha against head.sha to decide whether a version bump is required. That base SHA is the commit main pointed at when the PR was last synced, and it drifts behind main as other PRs merge. As a result the diff includes src/**/*.py files from already-merged PRs that the current branch never touched, so the check wrongly demands a version bump and a documentation-only PR ends up failing because unrelated source files appear in the comparison.

This changes the check to diff against the merge-base of the base and head commits (the point where the branch actually diverged from main) instead of the raw base ref.

Related Issue

N/A

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Dependency update

How to Test

Reproduced locally using the workflow's own logic against a drifted PR's real SHAs:

  1. Old behavior (raw base.sha): source files from already-merged PRs appear in the diff → check FAILS demanding a version bump.
  2. New behavior (merge-base): only the branch's own files appear → docs-only diff prints No source file changes under src/. Version bump not required. → check PASSES.
  3. Negative case preserved: a diff that genuinely contains a src/**/*.py change without a version bump still matches the regex and FAILS with the existing error — enforcement is unchanged for real source changes.

Also tested end-to-end : applying the fix to docs-only PR branch (#329) flipped Check Version Bump from failing to passing.

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Breaking Changes

None

Additional Notes

None

@jplbrun
jplbrun marked this pull request as ready for review September 11, 2026 22:35
@jplbrun
jplbrun requested a review from a team as a code owner September 11, 2026 22:35

@jeanscherf jeanscherf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great catch!

@jplbrun
jplbrun merged commit 478de39 into main Sep 14, 2026
10 checks passed
@jplbrun
jplbrun deleted the ci/version-bump-merge-base branch September 14, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants