Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add option to scripts-dev/lint.sh to only lint files changed since the last git commit #8472

Merged
merged 10 commits into from
Oct 15, 2020
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
1 change: 1 addition & 0 deletions changelog.d/8472.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `-d` option to `./scripts-dev/lint.sh` to lint files that have changed since the last git commit.
93 changes: 84 additions & 9 deletions scripts-dev/lint.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash
#
# Runs linting scripts over the local Synapse checkout
# isort - sorts import statements
Expand All @@ -7,15 +7,90 @@

set -e

if [ $# -ge 1 ]
then
files=$*
usage() {
echo
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
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 with a file extension that is not '.py' will be excluded."
echo "-h"
echo " Display this help text."
}

USING_DIFF=0
files=()

while getopts ":dh" opt; do
case $opt in
d)
USING_DIFF=1
;;
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))"

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+=("$@")
Comment on lines +68 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it feels to me like trying to mix -d and command-line parameters should be rejected: mixing the two seems confusing.

Copy link
Member Author

@anoadragon453 anoadragon453 Oct 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose there's not much use-case for it... though I'm not sure if we explicitly need to block it either. There may a random case where someone might want to do so.


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
Comment on lines +71 to +73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should stick to one of [ ] and [[ ]] :-p

(There is a difference. I can never remember what it is without looking it up.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ ... ] is POSIX-compliant, whereas [[ ... ]] supports fancy bash stuff like &&, ||, ~= etc. 1

https://stackoverflow.com/questions/13542832/difference-between-single-and-double-square-brackets-in-bash

We can't make the whole script POSIX-compliant however due to our heavy use of arrays, which are undefined in POSIX.

I have changed things to be more consistent in e1abece7c though.

# Then print and exit
echo "No files found to lint."
exit 0
fi
else
files="synapse tests scripts-dev scripts contrib synctl"
# 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" "setup.py")
fi
fi

echo "Linting these locations: $files"
isort $files
python3 -m black $files
echo "Linting these paths: ${files[*]}"
echo

# Print out the commands being run
set -x

isort "${files[@]}"
python3 -m black "${files[@]}"
./scripts-dev/config-lint.sh
flake8 $files
flake8 "${files[@]}"
4 changes: 1 addition & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__))

Expand Down