From 64984f5bf4b40109d8fde1eb23aef58ce62ef81c Mon Sep 17 00:00:00 2001 From: Joshua Pan Date: Fri, 21 Apr 2017 14:07:06 -0700 Subject: [PATCH] Rename tools/lint-all to tools/lint. Fixes #4574. --- docs/code-style.md | 4 ++-- docs/linters.md | 22 +++++++++++----------- docs/testing.md | 4 ++-- docs/translating.md | 2 +- tools/lib/provision.py | 2 +- tools/{lint-all => lint} | 28 ++++++++++++++-------------- tools/pre-commit | 8 ++++---- tools/test-all | 2 +- tools/travis/backend | 2 +- 9 files changed, 37 insertions(+), 37 deletions(-) rename tools/{lint-all => lint} (97%) diff --git a/docs/code-style.md b/docs/code-style.md index 261731cefd..3b40abf92b 100644 --- a/docs/code-style.md +++ b/docs/code-style.md @@ -13,7 +13,7 @@ question. You can run them all at once with - ./tools/lint-all + ./tools/lint You can set this up as a local Git commit hook with @@ -21,7 +21,7 @@ You can set this up as a local Git commit hook with The Vagrant setup process runs this for you. -`lint-all` runs many lint checks in parallel, including +`lint` runs many lint checks in parallel, including - JavaScript ([ESLint](http://eslint.org/)) - Python ([Pyflakes](http://pypi.python.org/pypi/pyflakes)) diff --git a/docs/linters.md b/docs/linters.md index 89042eeec8..b065c14df0 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -35,7 +35,7 @@ one small exception: it does not run mypy against scripts). You can also run them individually: - ./tools/lint-all + ./tools/lint ./tools/run-mypy ./tools/run-mypy --scripts-only @@ -49,7 +49,7 @@ Our linting tools generally support the ability to lint files individually--with some caveats--and those options will be described later in this document. -We may eventually bundle `run-mypy` into `lint-all`, but mypy is pretty +We may eventually bundle `run-mypy` into `lint`, but mypy is pretty resource intensive compared to the rest of the linters, because it does static code analysis. So we keep mypy separate to allow folks to quickly run the other lint checks. @@ -59,7 +59,7 @@ the other lint checks. Once you have read the [Zulip coding guidelines](code-style.html), you can be pretty confident that 99% of the code that you write will pass through the linters fine, as long as you are thorough about keeping your code clean. -And, of course, for minor oversights, `lint-all` is your friend, not your foe. +And, of course, for minor oversights, `lint` is your friend, not your foe. Occasionally, our linters will complain about things that are more of an artifact of the linter limitations than any actual problem with your @@ -81,7 +81,7 @@ describes our test system in detail. ## Lint checks -Most of our lint checks get performed by `./tools/lint-all`. These include the +Most of our lint checks get performed by `./tools/lint`. These include the following checks: - Check Python code with pyflakes. @@ -99,25 +99,25 @@ code analysis of Python type annotations throughout our Python codebase. Our [documentation on using mypy](mypy.html) covers mypy in more detail. -The rest of this document pertains to the checks that occur in `./tools/lint-all`. +The rest of this document pertains to the checks that occur in `./tools/lint`. -## lint-all +## lint -Zulip has a script called `lint-all` that lives in our "tools" directory. +Zulip has a script called `lint` that lives in our "tools" directory. It is the workhorse of our linting system, although in some cases it dispatches the heavy lifting to other components such as pyflakes, eslint, and other home grown tools. -You can find the source code [here](https://github.com/zulip/zulip/blob/master/tools/lint-all). +You can find the source code [here](https://github.com/zulip/zulip/blob/master/tools/lint). -In order for our entire lint suite to run in a timely fashion, the `lint-all` +In order for our entire lint suite to run in a timely fashion, the `lint` script performs several lint checks in parallel by forking out subprocesses. This mechanism is still evolving, but you can look at the method `run_parallel` to get the gist of how it works. ### Special options -You can use the `-h` option for `lint-all` to see its usage. One particular +You can use the `-h` option for `lint` to see its usage. One particular flag to take note of is the `--modified` flag, which enables you to only run lint checks against files that are modified in your git repo. Most of the "sub-linters" respect this flag, but some will continue to process all the files. @@ -156,7 +156,7 @@ unused imports--because of the way mypy type annotations work in Python 2, it would be inconvenient to enforce this too strictly.) Zulip also has custom regex-based rules that it applies to Python code. -Look for `python_rules` in the source code for `lint-all`. Note that we +Look for `python_rules` in the source code for `lint`. Note that we provide a mechanism to exclude certain lines of codes from these checks. Often, it is simply the case that our regex approach is too crude to correctly exonerate certain valid constructs. In other cases, the code diff --git a/docs/testing.md b/docs/testing.md index 8d8e45752c..c8e58cf0dd 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -32,13 +32,13 @@ Then, to run the full Zulip test suite, do this: ./tools/test-all ``` -This runs the linter (`tools/lint-all`) plus all of our test suites; +This runs the linter (`tools/lint`) plus all of our test suites; they can all be run separately (just read `tools/test-all` to see them). You can also run individual tests which can save you a lot of time debugging a test failure, e.g.: ``` -./tools/lint-all # Runs all the linters in parallel +./tools/lint # Runs all the linters in parallel ./tools/test-backend zerver.tests.test_bugdown.BugdownTest.test_inline_youtube ./tools/test-backend BugdownTest # Run `test-backend --help` for more options ./tools/test-js-with-casper 09-navigation.js diff --git a/docs/translating.md b/docs/translating.md index 3ff45c9ef9..a70dcbff73 100644 --- a/docs/translating.md +++ b/docs/translating.md @@ -254,7 +254,7 @@ JsonableError(_('English Text')) ``` To ensure we always internationalize our JSON errors messages, the -Zulip linter (`tools/lint-all`) checks for correct usage. +Zulip linter (`tools/lint`) checks for correct usage. ## Frontend translations diff --git a/tools/lib/provision.py b/tools/lib/provision.py index 98d104393f..4909f5b9a6 100755 --- a/tools/lib/provision.py +++ b/tools/lib/provision.py @@ -119,7 +119,7 @@ UBUNTU_COMMON_APT_DEPENDENCIES = [ "yui-compressor", "wget", "ca-certificates", # Explicit dependency in case e.g. wget is already installed - "puppet", # Used by lint-all + "puppet", # Used by lint "gettext", # Used by makemessages i18n "curl", # Used for fetching PhantomJS as wget occasionally fails on redirects "netcat", # Used for flushing memcached diff --git a/tools/lint-all b/tools/lint similarity index 97% rename from tools/lint-all rename to tools/lint index 499745a17f..50a39fa409 100755 --- a/tools/lint-all +++ b/tools/lint @@ -288,21 +288,21 @@ def build_custom_checkers(by_lang): {'pattern': '[.]text\(["\'][a-zA-Z]', 'description': 'Strings passed to $().text should be wrapped in i18n.t() for internationalization'}, {'pattern': 'compose_error\(["\']', - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), 'description': 'Argument to compose_error should be a literal string enclosed ' 'by i18n.t()'}, {'pattern': 'ui.report_success\(', - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), 'description': 'Deprecated function, use ui_report.success.'}, {'pattern': 'report.success\(["\']', - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), 'description': 'Argument to report_success should be a literal string enclosed ' 'by i18n.t()'}, {'pattern': 'ui.report_error\(', - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), 'description': 'Deprecated function, use ui_report.error.'}, {'pattern': 'report.error\(["\']', - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), 'description': 'Argument to report_error should be a literal string enclosed ' 'by i18n.t()'}, ]) + whitespace_rules @@ -312,7 +312,7 @@ def build_custom_checkers(by_lang): {'pattern': '".*"%\([a-z_].*\)?$', 'description': 'Missing space around "%"'}, {'pattern': "'.*'%\([a-z_].*\)?$", - 'exclude': set(['tools/lint-all', + 'exclude': set(['tools/lint', 'analytics/lib/counts.py', 'analytics/tests/test_counts.py', ]), @@ -336,14 +336,14 @@ def build_custom_checkers(by_lang): {'pattern': "assertEquals[(]", 'description': 'Use assertEqual, not assertEquals (which is deprecated).'}, {'pattern': "== None", - 'exclude': 'tools/lint-all', + 'exclude': 'tools/lint', 'description': 'Use `is None` to check whether something is None'}, {'pattern': "type:[(]", 'description': 'Missing whitespace after ":" in type annotation'}, {'pattern': "# type [(]", 'description': 'Missing : after type in type annotation'}, {'pattern': "#type", - 'exclude': 'tools/lint-all', + 'exclude': 'tools/lint', 'description': 'Missing whitespace after "#" in type annotation'}, {'pattern': 'if[(]', 'description': 'Missing space between if and ('}, @@ -368,10 +368,10 @@ def build_custom_checkers(by_lang): 'description': 'Use json_success() to return nothing'}, # To avoid json_error(_variable) and json_error(_(variable)) {'pattern': '\Wjson_error\(_\(?\w+\)', - 'exclude': set(['tools/lint-all', 'zerver/tests']), + 'exclude': set(['tools/lint', 'zerver/tests']), 'description': 'Argument to json_error should be a literal string enclosed by _()'}, {'pattern': '\Wjson_error\([\'"].+[),]$', - 'exclude': set(['tools/lint-all', 'zerver/tests']), + 'exclude': set(['tools/lint', 'zerver/tests']), 'exclude_line': set([ # We don't want this string tagged for translation. ('zerver/views/compatibility.py', 'return json_error("Client is too old")'), @@ -379,10 +379,10 @@ def build_custom_checkers(by_lang): 'description': 'Argument to json_error should a literal string enclosed by _()'}, # To avoid JsonableError(_variable) and JsonableError(_(variable)) {'pattern': '\WJsonableError\(_\(?\w.+\)', - 'exclude': set(['tools/lint-all', 'zerver/tests']), + 'exclude': set(['tools/lint', 'zerver/tests']), 'description': 'Argument to JsonableError should be a literal string enclosed by _()'}, {'pattern': '\WJsonableError\(["\'].+\)', - 'exclude': set(['tools/lint-all', 'zerver/tests']), + 'exclude': set(['tools/lint', 'zerver/tests']), 'description': 'Argument to JsonableError should be a literal string enclosed by _()'}, {'pattern': '([a-zA-Z0-9_]+)=REQ\([\'"]\\1[\'"]', 'description': 'REQ\'s first argument already defaults to parameter name'}, @@ -434,7 +434,7 @@ def build_custom_checkers(by_lang): }, {'pattern': 'render_to_response\(', 'description': "Use render() instead of render_to_response().", - 'exclude': set(['tools/lint-all']), + 'exclude': set(['tools/lint']), }, # This rule might give false positives in virtualenv setup files which should be excluded, # and comments which should be rewritten to avoid use of "python2", "python3", etc. @@ -442,7 +442,7 @@ def build_custom_checkers(by_lang): 'exclude': set(['tools/lib/provision.py', 'tools/setup/setup_venvs.py', 'scripts/lib/setup_venv.py', - 'tools/lint-all']), + 'tools/lint']), 'description': 'Explicit python invocations should not include a version'} ]) + whitespace_rules bash_rules = [ diff --git a/tools/pre-commit b/tools/pre-commit index 0120a4d1a4..035ebee0d7 100755 --- a/tools/pre-commit +++ b/tools/pre-commit @@ -1,6 +1,6 @@ #!/bin/bash -# This hook runs the Zulip code linter ./tools/lint-all and returns true +# This hook runs the Zulip code linter ./tools/lint and returns true # regardless of linter results so that your commit may continue. # Messages from the linter will be printed out to the screen. @@ -16,10 +16,10 @@ if [ -z "$changed_files" ]; then fi if [ -z "$VIRTUAL_ENV" ] && `which vagrant > /dev/null` && [ -e .vagrant ]; then - vcmd="/srv/zulip/tools/lint-all --pep8 --no-gitlint --force $changed_files || true" - echo "Running lint-all using vagrant..." + vcmd="/srv/zulip/tools/lint --pep8 --no-gitlint --force $changed_files || true" + echo "Running lint using vagrant..." vagrant ssh -c "$vcmd" else - ./tools/lint-all --pep8 --no-gitlint --force $changed_files || true + ./tools/lint --pep8 --no-gitlint --force $changed_files || true fi exit 0 diff --git a/tools/test-all b/tools/test-all index 7dacc0a668..ea839a19c8 100755 --- a/tools/test-all +++ b/tools/test-all @@ -33,7 +33,7 @@ function run { run ./tools/check-provision $FORCEARG run ./tools/clean-repo run ./tools/test-tools -run ./tools/lint-all --pep8 $FORCEARG +run ./tools/lint --pep8 $FORCEARG run ./tools/test-migrations run ./tools/test-js-with-node run ./tools/run-mypy diff --git a/tools/travis/backend b/tools/travis/backend index ac57154fca..1ea98d8fa4 100755 --- a/tools/travis/backend +++ b/tools/travis/backend @@ -5,7 +5,7 @@ source tools/travis/activate-venv set -e set -x -./tools/lint-all --pep8 # Include the slow and thus non-default pep8 linter check +./tools/lint --pep8 # Include the slow and thus non-default pep8 linter check ./tools/test-tools ./tools/test-backend --coverage ./tools/test-migrations