From 0e2973d004ed5368785438b54aac1d95c32f42ac Mon Sep 17 00:00:00 2001 From: Christopher Cooper Date: Fri, 25 Oct 2024 18:02:58 -0700 Subject: [PATCH 1/3] [dev] restrict pylint to changed files --- format.sh | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/format.sh b/format.sh index 66b966c3029..f210f370182 100755 --- a/format.sh +++ b/format.sh @@ -60,6 +60,10 @@ BLACK_INCLUDES=( 'sky/skylet/providers/ibm' ) +PYLINT_FLAGS=( + '--load-plugins' 'pylint_quotes' +) + # Format specified files format() { yapf --in-place "${YAPF_FLAGS[@]}" "$@" @@ -77,7 +81,7 @@ format_changed() { MERGEBASE="$(git merge-base origin/master HEAD)" if ! git diff --diff-filter=ACM --quiet --exit-code "$MERGEBASE" -- '*.py' '*.pyi' &>/dev/null; then - git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' '*.pyi' | xargs -P 5 \ + git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' '*.pyi' | xargs -P 5 -d '\n' \ yapf --in-place "${YAPF_EXCLUDES[@]}" "${YAPF_FLAGS[@]}" fi @@ -119,7 +123,21 @@ mypy $(cat tests/mypy_files.txt) # Run Pylint echo 'Sky Pylint:' -pylint --load-plugins pylint_quotes sky +if [[ "$1" == '--files' ]]; then + # If --files is passed, filter to files within sky/ and pass to pylint. + pylint "${PYLINT_FLAGS[@]}" "${@:2}" +elif [[ "$1" == '--all' ]]; then + # Pylint entire sky directory. + pylint "${PYLINT_FLAGS[@]}" sky +else + # Pylint only files in sky/ that have changed in last commit. + changed_files=$(git diff --name-only --diff-filter=ACM "$MERGEBASE" -- 'sky/**/*.py' 'sky/**/*.pyi') + if [[ -n "$changed_files" ]]; then + echo "$changed_files" | xargs -d '\n' pylint "${PYLINT_FLAGS[@]}" + else + echo 'Pylint skipped: no files changed in sky/.' + fi +fi if ! git diff --quiet &>/dev/null; then echo 'Reformatted files. Please review and stage the changes.' From 30646474e44513efbdc8b4695a7dcc9939d4c0a9 Mon Sep 17 00:00:00 2001 From: Christopher Cooper Date: Fri, 25 Oct 2024 18:18:22 -0700 Subject: [PATCH 2/3] fix glob --- format.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/format.sh b/format.sh index f210f370182..741126dc87b 100755 --- a/format.sh +++ b/format.sh @@ -131,7 +131,7 @@ elif [[ "$1" == '--all' ]]; then pylint "${PYLINT_FLAGS[@]}" sky else # Pylint only files in sky/ that have changed in last commit. - changed_files=$(git diff --name-only --diff-filter=ACM "$MERGEBASE" -- 'sky/**/*.py' 'sky/**/*.pyi') + changed_files=$(git diff --name-only --diff-filter=ACM "$MERGEBASE" -- 'sky/*.py' 'sky/*.pyi') if [[ -n "$changed_files" ]]; then echo "$changed_files" | xargs -d '\n' pylint "${PYLINT_FLAGS[@]}" else From c869343fb92826a08da5f198a5eba4dd2860ef9e Mon Sep 17 00:00:00 2001 From: Christopher Cooper Date: Mon, 28 Oct 2024 14:11:39 -0700 Subject: [PATCH 3/3] avoid use of xargs -d --- format.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/format.sh b/format.sh index 741126dc87b..b06481b4c10 100755 --- a/format.sh +++ b/format.sh @@ -81,8 +81,9 @@ format_changed() { MERGEBASE="$(git merge-base origin/master HEAD)" if ! git diff --diff-filter=ACM --quiet --exit-code "$MERGEBASE" -- '*.py' '*.pyi' &>/dev/null; then - git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' '*.pyi' | xargs -P 5 -d '\n' \ - yapf --in-place "${YAPF_EXCLUDES[@]}" "${YAPF_FLAGS[@]}" + git diff --name-only --diff-filter=ACM "$MERGEBASE" -- '*.py' '*.pyi' | \ + tr '\n' '\0' | xargs -P 5 -0 \ + yapf --in-place "${YAPF_EXCLUDES[@]}" "${YAPF_FLAGS[@]}" fi } @@ -133,7 +134,7 @@ else # Pylint only files in sky/ that have changed in last commit. changed_files=$(git diff --name-only --diff-filter=ACM "$MERGEBASE" -- 'sky/*.py' 'sky/*.pyi') if [[ -n "$changed_files" ]]; then - echo "$changed_files" | xargs -d '\n' pylint "${PYLINT_FLAGS[@]}" + echo "$changed_files" | tr '\n' '\0' | xargs -0 pylint "${PYLINT_FLAGS[@]}" else echo 'Pylint skipped: no files changed in sky/.' fi