Skip to content

Commit

Permalink
Always use branches (not SHAs) for formatting workflow
Browse files Browse the repository at this point in the history
The SHAs that GitHub provides might be outdated, so we'd see lots more changed
files than are actually in the given PR.

By using the branch name (and fetching branches after cloning, for good
measure), we should always get the latest commits in question.
  • Loading branch information
TimoWilken committed Jan 19, 2024
1 parent 4dbbe62 commit 2d84954
Showing 1 changed file with 16 additions and 10 deletions.
26 changes: 16 additions & 10 deletions .github/workflows/c++-code-formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ name: C++ code formatting reusable workflow
permissions: {}

env:
# GitHub also provides github.event.pull_request.{head,base}.sha, but these
# aren't always the latest commits on their respective branches (e.g. see
# https://github.com/AliceO2Group/AliceO2/pull/12499). Using them might lead
# to false positives in the errors we show.
BASE_BRANCH: ${{ github.event.pull_request.base.ref }}
PR_BRANCH: ${{ github.event.pull_request.head.ref }}
PR_NUMBER: ${{ github.event.pull_request.number }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
BASE_SHA: ${{ github.event.pull_request.base.sha }}

jobs:
clang-format:
Expand Down Expand Up @@ -61,14 +63,13 @@ jobs:
id: clang_format
run: |
set -x
# $BASE_SHA is the latest commit on the branch the PR will be merged
# into, NOT the commit this PR derives from! For that, we need to find
# the latest common ancestor between the PR and the branch we are
# merging into.
base_commit=$(git merge-base HEAD "$BASE_SHA")
# $BASE_BRANCH is the branch the PR will be merged into, NOT the
# commit this PR derives from! For that, we need to find the latest
# common ancestor between the PR and the branch we are merging into.
base_commit=$(git merge-base HEAD "$BASE_BRANCH")
# Find changed files, ignoring binary files.
readarray -d '' commit_files < \
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA")
<(git diff -z --diff-filter d --name-only "$base_commit")
[ ${#commit_files[@]} -gt 0 ] || { echo "No files to check"; exit 0; }
# Check for invalid file extensions for C++ code.
Expand Down Expand Up @@ -156,6 +157,11 @@ jobs:
# many commits back that point is.
fetch-depth: 0

# Fetch the PR's head commit to find the common ancestor.
- name: Fetch PR branch
run: |
git fetch origin "$BASE_BRANCH" "pull/$PR_NUMBER/head:$PR_BRANCH"
- name: Check copyright headers
env:
# The expected copyright notice. Comment markers ("//" or "#") are
Expand All @@ -177,7 +183,7 @@ jobs:
# Find changed C++ and CMake files. Keep the file extensions in sync
# with the ones in the "case" statement below!
readarray -d '' files < \
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA" \
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_BRANCH" \
-- '*.cxx' '*.h' '*.C' '*.cmake' '*/CMakeLists.txt')
# Run copyright notice check. Comment lines start with "//" for C++
# files and "#" for CMake files.
Expand Down Expand Up @@ -314,7 +320,7 @@ jobs:
run: |
# Find changed files, ignoring binary files.
readarray -d '' files < \
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_SHA" |
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_BRANCH" |
while read -rd '' filename; do
file -bi "$filename" | grep -q charset=binary ||
printf "%s\\0" "$filename"
Expand Down

0 comments on commit 2d84954

Please sign in to comment.