Skip to content

Commit

Permalink
Refactor the lint action into a build script, temporarily update .cla…
Browse files Browse the repository at this point in the history
…ng-format (#183)

Two changes in this PR:

1) Refactor the lint action to be a locally runnable bash script. This should give _more_ predictable results in terms of addressing CI failures.

2) Update `.clang-format` to use Google style in the short term. More on this rationale below.

Background on all this is that my editor formatted a large chunk of a file which caused the lint action to fail remotely, and took me quite some time to roll back the individual formatting changes, since google styling appears to prefer the most common spacing of `Type* var` vs `Type *var` in a file as the proper formatting.

This change will allow local qualification / fixing, as well as provide a stop-gap on editor formatting issues while #15 is discussed. While I agree that we should resolve #15, I think more discussion is required, and there is unnecessary friction in the current setup/CI of clang-format in the current repo that should be addressed. Happy to drive the formatting conversation in the near future, but think this should be in place in the short term.

GH Actions run testing that it errors properly in CI: https://github.com/GleasonK/stablehlo/actions/runs/3106772722/jobs/5034066123

Closes #75
  • Loading branch information
GleasonK authored Sep 23, 2022
1 parent fe8c9b4 commit ee50580
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .clang-format
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
BasedOnStyle: LLVM
BasedOnStyle: Google
AlwaysBreakTemplateDeclarations: Yes
16 changes: 6 additions & 10 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,20 @@

name: Lint

on: [pull_request]
on:
pull_request:
paths-ignore: ['**.md', 'docs/**']

jobs:
clang-format:
# This job can only be run on pull_request since GITHUB_BASE_REF is only set on PR.
if: "github.event_name == 'pull_request'"
runs-on: ubuntu-latest
steps:
- name: Installing dependencies
run: |
wget https://raw.githubusercontent.com/llvm-mirror/clang/master/tools/clang-format/git-clang-format -O /tmp/git-clang-format
chmod +x /tmp/git-clang-format
- name: Checking out repository
uses: actions/checkout@v2
- name: Fetching Base Branch
# We have to explicitly fetch the base branch as well
run: git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"

- name: Running clang-format on changed source files
run: |
/tmp/git-clang-format "${GITHUB_BASE_REF?}" --style=google
git diff --exit-code
git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
./build_tools/github_actions/lint_clang_format.sh -b "${GITHUB_BASE_REF}"
50 changes: 50 additions & 0 deletions build_tools/github_actions/lint_clang_format.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Copyright 2022 The StableHLO Authors.
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

print_usage() {
echo "Usage: $0 [-fd]"
echo " -f Auto-fix clang-format issues."
echo " -b <branch> Base branch name, default to origin/main."
}

FORMAT_MODE='validate'
BASE_BRANCH="$(git merge-base HEAD origin/main)"
while getopts 'fb:' flag; do
case "${flag}" in
f) FORMAT_MODE="fix" ;;
b) BASE_BRANCH="$OPTARG" ;;
*) print_usage
exit 1 ;;
esac
done
shift $(( OPTIND - 1 ))

if [[ $# -ne 0 ]] ; then
print_usage
exit 1
fi

echo "Gathering changed files..."
CHANGED_FILES=$(git diff --name-only HEAD $BASE_BRANCH | grep '.*\.h\|.*\.cpp' | xargs)
if [[ -z "$CHANGED_FILES" ]]; then
echo "No files to format."
exit 0
fi

echo "Running clang-format [mode=$FORMAT_MODE]..."
echo " Files: $CHANGED_FILES"
if [[ $FORMAT_MODE == 'fix' ]]; then
clang-format --style=google -i $CHANGED_FILES
else
clang-format --style=google --dry-run --Werror $CHANGED_FILES
fi

0 comments on commit ee50580

Please sign in to comment.