Skip to content

Commit

Permalink
Fix branch handling in formatting workflow
Browse files Browse the repository at this point in the history
We already checkout the PR's branch, so no need to fetch it again. That also
avoids conflicts with the base branch, if both are called the same.

Also, always refer to the remote's branch directly, then there's no need to
store the branch locally.
  • Loading branch information
TimoWilken committed Jan 19, 2024
1 parent 53b6f22 commit fb821b2
Showing 1 changed file with 13 additions and 15 deletions.
28 changes: 13 additions & 15 deletions .github/workflows/c++-code-formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ 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
# GitHub also provides github.event.pull_request.base.sha, but that isn't
# always the latest commit on the base branch (e.g. see
# https://github.com/AliceO2Group/AliceO2/pull/12499). Using it 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 }}
Expand Down Expand Up @@ -57,7 +57,7 @@ jobs:
run: |
git config --global user.email '[email protected]'
git config --global user.name 'ALICE Action Bot'
git fetch origin "$BASE_BRANCH:$BASE_BRANCH" "pull/$PR_NUMBER/head:$PR_BRANCH"
git fetch origin "$BASE_BRANCH"
- name: Run clang format
id: clang_format
Expand All @@ -66,7 +66,7 @@ jobs:
# $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")
base_commit=$(git merge-base HEAD "origin/$BASE_BRANCH")
# Find changed files, ignoring binary files.
readarray -d '' commit_files < \
<(git diff -z --diff-filter d --name-only "$base_commit")
Expand Down Expand Up @@ -157,10 +157,9 @@ 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:$BASE_BRANCH" "pull/$PR_NUMBER/head:$PR_BRANCH"
# Fetch the PR's base branch to find the common ancestor.
- name: Update PR branch
run: git fetch origin "$BASE_BRANCH"

- name: Check copyright headers
env:
Expand All @@ -183,7 +182,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_BRANCH" \
<(git diff -z --diff-filter d --name-only --merge-base "origin/$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 @@ -311,16 +310,15 @@ 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:$BASE_BRANCH" "pull/$PR_NUMBER/head:$PR_BRANCH"
# Fetch the PR's base branch to find the common ancestor.
- name: Update PR branch
run: git fetch origin "$BASE_BRANCH"

- name: Find bad spacing
run: |
# Find changed files, ignoring binary files.
readarray -d '' files < \
<(git diff -z --diff-filter d --name-only --merge-base "$BASE_BRANCH" |
<(git diff -z --diff-filter d --name-only --merge-base "origin/$BASE_BRANCH" |
while read -rd '' filename; do
file -bi "$filename" | grep -q charset=binary ||
printf "%s\\0" "$filename"
Expand Down

0 comments on commit fb821b2

Please sign in to comment.