Skip to content

Commit

Permalink
Make a few improvements to linting (#8184)
Browse files Browse the repository at this point in the history
Use our own script for the yamllint check. This allows us to only
look at modified files and run it locally as part of lint.sh

Made some misc cleanups to the other bash scripts along the way, like
making check_tabs.sh not hang forever if there are no modified files
(grep gets passed an empty list of files and then sits there waiting
for input).
  • Loading branch information
GMNGeoffrey committed Jan 26, 2022
1 parent 3d97314 commit cf9ee44
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 10 deletions.
13 changes: 7 additions & 6 deletions .github/workflows/lint.yml
Expand Up @@ -123,9 +123,10 @@ jobs:
yamllint:
runs-on: ubuntu-18.04
steps:
- uses: actions/checkout@v2
- name: yaml-lint
uses: ibiqlik/action-yamllint@v1
with:
strict: true
file_or_dir: .github/**/*.yml build_tools/buildkite/**/*.yml
- 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: yamllint
run: ./scripts/run_yamllint.sh
5 changes: 5 additions & 0 deletions scripts/check_tabs.sh
Expand Up @@ -18,6 +18,7 @@ declare -a excluded_files_patterns=(
"/third_party/"
"^third_party/"
"*Makefile*"
# Symlinks make grep upset
"^integrations/tensorflow/iree-dialects$"
)

Expand All @@ -28,6 +29,10 @@ readarray -t files < <(\
(git diff --name-only --diff-filter=d "${BASE_REF}" || kill $$) \
| grep -v -E "${excluded_files_pattern?}")

if (( ${#files[@]} == 0 )); then
exit 0
fi;

diff="$(grep --with-filename --line-number --perl-regexp --binary-files=without-match '\t' "${files[@]}")"

grep_exit="$?"
Expand Down
13 changes: 10 additions & 3 deletions scripts/lint.sh
Expand Up @@ -74,7 +74,7 @@ fi
echo "***** yapf *****"
# Don't fail script if condition is false
disable_update_ret
if exists yapf > /dev/null; then
if exists yapf; then
enable_update_ret
git diff -U0 main | ./third_party/format_diff/format_diff.py yapf -i
else
Expand Down Expand Up @@ -109,9 +109,16 @@ echo "***** tabs *****"
./scripts/check_tabs.sh

echo "***** yamllint *****"
echo "'yamllint' check not yet implemented. Skipping check"
disable_update_ret
if exists yamllint; then
enable_update_ret
./scripts/run_yamllint.sh
else
enable_update_ret
echo "'yamllint' not found. Skipping check"
fi

if [[ "${FINAL_RET}" -ne 0 ]]; then
if (( "${FINAL_RET}" != 0 )); then
echo "Encountered failures. Check error messages and changes to the working" \
"directory and git index (which may contain fixes) and try again."
fi
Expand Down
2 changes: 1 addition & 1 deletion scripts/run_buildifier.sh
Expand Up @@ -35,7 +35,7 @@ readarray -t files < <(\
| grep -E "${included_files_pattern?}" \
| grep -v -E "${excluded_files_pattern?}")

if [ ${#files[@]} -eq 0 ]; then
if (( ${#files[@]} == 0 )); then
echo "No Bazel files changed"
exit 0
fi
Expand Down
41 changes: 41 additions & 0 deletions scripts/run_yamllint.sh
@@ -0,0 +1,41 @@
#!/bin/bash

# Copyright 2021 The IREE Authors
#
# Licensed under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

# Runs yamllint on files modified vs the specified reference commit
# (default "main")

set -uo pipefail

BASE_REF="${1:-main}"

declare -a included_files_patterns=(
"\.yaml$"
"\.yml$"
)

declare -a excluded_files_patterns=(
"/third_party/"
"^third_party/"
)

# Join on |
included_files_pattern="$(IFS="|" ; echo "${included_files_patterns[*]?}")"
excluded_files_pattern="$(IFS="|" ; echo "${excluded_files_patterns[*]?}")"

readarray -t files < <(\
(git diff --name-only --diff-filter=d "${BASE_REF}" || kill $$) \
| grep -E "${included_files_pattern?}" \
| grep -v -E "${excluded_files_pattern?}")

if (( ${#files[@]} == 0 )); then
echo "No Yaml files changed"
exit 0
fi


yamllint --strict "${files[@]}"

0 comments on commit cf9ee44

Please sign in to comment.