commitchecker: refactor ancestry-path handling and add stale-branch detection#1
Merged
tmshort merged 6 commits intotmshort:debug-ancestry-pathfrom Mar 25, 2026
Conversation
The pattern `commitchecker` matches any file starting with that prefix, including test files like commits_test.go in subdirectories. Use `/commitchecker` to only match the built binary at the directory root.
The `if err != nil` check was always false because `err` was declared as `var err error` and never assigned before the check.
…ectCommitsBetween Split CommitsBetween (which always used --ancestry-path) into two explicit functions: - AllCommitsBetween: finds all non-merge commits reachable from end but not start - DirectCommitsBetween: same, but restricted to the direct ancestry path via --ancestry-path Both call a shared private gitLog() helper. The caller in main() is updated to call each explicitly instead of looping over CheckMode values, with shared error handling extracted into listCommits() and validation into validateCommits(). The error message for git log failures now includes the full command for easier copy-paste debugging.
When all reachable commits are found but none are on the direct ancestry path, fail with an error telling the user to rebase. This prevents false positive passes when a PR branch has fallen behind main. Also removes SameCommit, ErrNoCommitsFound, and CheckMode which are no longer used after the refactor.
TestStaleBranch creates a DAG where a PR branch forks from an early commit, main advances, then the PR is merged. Verifies that AllCommitsBetween finds the PR commits but DirectCommitsBetween does not, confirming the --ancestry-path behavior. TestUpToDateBranch verifies both functions return the same commits on a linear history.
Add a Limitations section to the README explaining stale-branch detection, the error message users will see, how to read CI logs, and how to fix it (rebase the PR). Add ancestry-path.md with a detailed explanation of --ancestry-path git semantics, why the commitchecker needs it, and how it can drop commits on stale branches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review feedback for openshift#114. Split into 6 commits for easier review:
/commitcheckerpattern was matching test files in subdirectoriesif err != nilinNewCommitFromOnelineLogAllCommitsBetween(no--ancestry-path) andDirectCommitsBetween(with--ancestry-path), both calling a sharedgitLog()helper. Restructuremain()into explicit calls withlistCommits()/validateCommits()helpersSameCommit,ErrNoCommitsFound,CheckModeTestStaleBranchandTestUpToDateBranchcreate temporary git repos to verify the--ancestry-pathbehaviorancestry-path.mddeep-dive