From 6a8bc7684fee93a08dd4380979b278304bc72c96 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 6 Oct 2020 14:25:47 +0100 Subject: [PATCH 01/10] Add -d option to ./scripts-dev/lint.sh to only lint changed files --- scripts-dev/lint.sh | 87 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 10 deletions(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 064799365832..86b73aa13175 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # # Runs linting scripts over the local Synapse checkout # isort - sorts import statements @@ -7,15 +7,82 @@ set -e -if [ $# -ge 1 ] -then - files=$* -else - files="synapse tests scripts-dev scripts contrib synctl" +usage() { + echo + echo "Usage: $0 [-h] [-d] [paths...]" + echo + echo "-d" + echo " Lint files that have changed since the last git commit." + echo + echo " If paths are provided and this option is set, both provided paths and those" + echo " that have changed since the last commit will be linted." + echo + echo " If no paths are provided and this option is not set, all files will be linted." + echo + echo " Note that paths will be excluded if they both have a file extension, and it is not 'py'." + echo "-h" + echo " Display this help text." +} + +USING_DIFF=0 +files=() + +while getopts ":dh" opt; do + case $opt in + d) + USING_DIFF=1 + + # Check both staged and non-staged changes + for path in $(git diff HEAD --name-only); do + filename=$(basename "$path") + file_extension="${filename##*.}" + + # If an extension is present, and it's something other than 'py', + # then ignore this file + if [[ -n ${file_extension+x} && $file_extension != "py" ]]; then + continue + fi + + # Append this path to our list of files to lint + files+=("$path") + done + ;; + h) + usage + exit + ;; + \?) + echo "ERROR: Invalid option: -$OPTARG" >&2 + usage + exit + ;; + esac +done + +# Strip any options from the command line arguments now that +# we've finished processing them +shift "$((OPTIND-1))" + +# Append any remaining arguments as files to lint +files+=("$@") + +# If we were not asked to lint changed files, and no paths were found as a result, +# then lint everything! +if [[ $USING_DIFF -eq 0 && -z ${files+x} ]]; then + # Lint all source code files and directories + files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl") fi -echo "Linting these locations: $files" -isort $files -python3 -m black $files +echo "Linting these paths:" +for path in "${files[@]}"; do + echo "- \"$path\"" +done +echo + +# Print out the commands being run +set -x + +isort "${files[@]}" +python3 -m black "${files[@]}" ./scripts-dev/config-lint.sh -flake8 $files +flake8 "${files[@]}" From 4d077bb0a71c0ca129bde6c0331d24ba472c0459 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 6 Oct 2020 14:27:35 +0100 Subject: [PATCH 02/10] Update CONTRIBUTING with new lint.sh features --- CONTRIBUTING.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 524f82433dba..c17e3b23995a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,6 +63,10 @@ run-time: ./scripts-dev/lint.sh path/to/file1.py path/to/file2.py path/to/folder ``` +You can also provided the `-d` option, which will lint the files that have been +changed since the last git commit. This will often be significantly faster than +linting the whole codebase. + Before pushing new changes, ensure they don't produce linting errors. Commit any files that were corrected. From 36b3cc82b2e242ed5e4ebcc5462e8988b0daaec0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 6 Oct 2020 14:39:47 +0100 Subject: [PATCH 03/10] Changelog --- changelog.d/8472.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8472.feature diff --git a/changelog.d/8472.feature b/changelog.d/8472.feature new file mode 100644 index 000000000000..880f3f5e14fa --- /dev/null +++ b/changelog.d/8472.feature @@ -0,0 +1 @@ +Add `-d` option to `./scripts-dev/lint.sh` to lint files that have changed since the last git commit. \ No newline at end of file From 59ebd677b70108f9c2a706abedf834cb28460e65 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 8 Oct 2020 11:09:31 +0100 Subject: [PATCH 04/10] Print linted paths on a single line separated by spaces --- scripts-dev/lint.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 86b73aa13175..527aa89db17d 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -73,10 +73,7 @@ if [[ $USING_DIFF -eq 0 && -z ${files+x} ]]; then files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl") fi -echo "Linting these paths:" -for path in "${files[@]}"; do - echo "- \"$path\"" -done +echo "Linting these paths: ${files[*]}" echo # Print out the commands being run From 06226ef95378d733a2345d700d45f700149b9309 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 12 Oct 2020 10:17:41 +0100 Subject: [PATCH 05/10] Print and exit if no changed files were found while using -d --- scripts-dev/lint.sh | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 527aa89db17d..537545839ee0 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -66,11 +66,20 @@ shift "$((OPTIND-1))" # Append any remaining arguments as files to lint files+=("$@") -# If we were not asked to lint changed files, and no paths were found as a result, -# then lint everything! -if [[ $USING_DIFF -eq 0 && -z ${files+x} ]]; then - # Lint all source code files and directories - files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl") +if [[ $USING_DIFF -eq 1 ]]; then + # If we were asked to lint changed files, and no paths were found as a result... + if [ ${#files[@]} -eq 0 ]; then + # Then print and exit + echo "No files found to lint." + exit 0 + fi +else + # If we were not asked to lint changed files, and no paths were found as a result, + # then lint everything! + if [[ -z ${files+x} ]]; then + # Lint all source code files and directories + files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl") + fi fi echo "Linting these paths: ${files[*]}" From 16c2f546184cae4d3cb40718563922f943f9d5b7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 12 Oct 2020 10:18:38 +0100 Subject: [PATCH 06/10] feature -> misc --- changelog.d/{8472.feature => 8472.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{8472.feature => 8472.misc} (100%) diff --git a/changelog.d/8472.feature b/changelog.d/8472.misc similarity index 100% rename from changelog.d/8472.feature rename to changelog.d/8472.misc From e51baaef0142cbfebb297ca02ad8b103d67c3d3e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 12 Oct 2020 10:19:27 +0100 Subject: [PATCH 07/10] Clearer help text --- scripts-dev/lint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 537545839ee0..6a06811db402 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -19,7 +19,7 @@ usage() { echo echo " If no paths are provided and this option is not set, all files will be linted." echo - echo " Note that paths will be excluded if they both have a file extension, and it is not 'py'." + echo " Note that paths with a file extension that is not '.py' will be excluded." echo "-h" echo " Display this help text." } From 5fb02c9cffeb0891013341081063b674dd13105e Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 15 Oct 2020 14:16:28 +0100 Subject: [PATCH 08/10] Lint setup.py as well. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- scripts-dev/lint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 6a06811db402..7b1e1a3dc609 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -78,7 +78,7 @@ else # then lint everything! if [[ -z ${files+x} ]]; then # Lint all source code files and directories - files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl") + files=("synapse" "tests" "scripts-dev" "scripts" "contrib" "synctl" "setup.py") fi fi From 90d195fed947446da36997b313c6271239f64344 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 15 Oct 2020 14:17:38 +0100 Subject: [PATCH 09/10] Fix linting issues in setup.py --- setup.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 926b1bc86fa8..08843fe2a3e4 100755 --- a/setup.py +++ b/setup.py @@ -15,12 +15,10 @@ # 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. - import glob import os -from setuptools import setup, find_packages, Command -import sys +from setuptools import Command, find_packages, setup here = os.path.abspath(os.path.dirname(__file__)) From 0056945a5baeed662d4e3b926b68e0f72ec5706a Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 15 Oct 2020 14:20:54 +0100 Subject: [PATCH 10/10] Move diff logic outside of the getopts loop --- scripts-dev/lint.sh | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/scripts-dev/lint.sh b/scripts-dev/lint.sh index 7b1e1a3dc609..f2b65a210533 100755 --- a/scripts-dev/lint.sh +++ b/scripts-dev/lint.sh @@ -31,21 +31,6 @@ while getopts ":dh" opt; do case $opt in d) USING_DIFF=1 - - # Check both staged and non-staged changes - for path in $(git diff HEAD --name-only); do - filename=$(basename "$path") - file_extension="${filename##*.}" - - # If an extension is present, and it's something other than 'py', - # then ignore this file - if [[ -n ${file_extension+x} && $file_extension != "py" ]]; then - continue - fi - - # Append this path to our list of files to lint - files+=("$path") - done ;; h) usage @@ -63,6 +48,23 @@ done # we've finished processing them shift "$((OPTIND-1))" +if [ $USING_DIFF -eq 1 ]; then + # Check both staged and non-staged changes + for path in $(git diff HEAD --name-only); do + filename=$(basename "$path") + file_extension="${filename##*.}" + + # If an extension is present, and it's something other than 'py', + # then ignore this file + if [[ -n ${file_extension+x} && $file_extension != "py" ]]; then + continue + fi + + # Append this path to our list of files to lint + files+=("$path") + done +fi + # Append any remaining arguments as files to lint files+=("$@")