docs: use GH base sha to lint PR commits #4198
Merged
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.
Description
The current documentation suggests using
HEAD~n
to go through all PR commits wheren
is the number of commits in the PR. This may not always produce the desired result though, especially if the history is not perfectly linear and there is a complex history of merges.GitHub Actions provides the base sha on the
pull_request
object as well. This should give a revision list in line with the git log and revisions shown on GitHub.Motivation and Context
While using the previous CI workflow, I found that the commits being linted where different from those I was seeing in the PR. After reading a bit about how
~
and^
work with git I came to the conclusion that going~n
will not necessarily point to the same revision as the first commit shown in the PR (merge base), especially if the history is complex.These are my findings. If there's something I've overlooked or you think my conclusions are incorrect, please do let me know!
The original discussion on the GitHub docs seems to come from here: #586.
Usage examples
In a GitHub Action:
How Has This Been Tested?
I compared the commits linted by using each method. The commits linted by the existing method included some not shown in the PR. The commits linted by the new method matched exactly.
Types of changes
Checklist: