Workflow autofixes #195

petemounce · 2020-10-20T22:32:49Z

Changes

The automatic fixes applied via pre-commit run --all-files, stacked on #194.

I don't think I'd land this in one go, particularly; this is more illustrative. I also probably would choose to not land this until after CI runs pre-commit for incremental linting.

Verification

verifying internally, incrementally. shfmt #199 and Workflow autofix yaml #200 were checked by @improbable-mattchurch yesterday - no p4-related errors.

improbable-prow-robot · 2020-10-20T22:32:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ca-johnson

Can we leave the " vs ' alone?

I have generally followed convention of using ' for "programmer" values and " for "user-facing" values (e.g. messages)

There are also some bits of formatting I'm not particularly impressed with where it breaks some lines across multiple lines in a way which makes them more difficult to read (particularly list comprehensions)

Also worth mentioning that this does cause a lot of noise in git blame which generally provides more value than some of the less valuable lint changes

dev/setup_env_osx.sh

ca-johnson · 2020-10-21T15:13:43Z

Looks a lot better with this config 👍 thanks

petemounce · 2020-10-22T12:39:52Z

@ca-johnson this is now just the python black autoformat. From here, we have this lot remaining:

$ pre-commit run --all-files
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 2

.buildkite/local-pipeline.yml:16:3: [warning] comment not indented like content (comments-indentation)

Format YAML files........................................................Passed
Check for added large files..............................................Passed
check BOM - deprecated: use fix-byte-order-marker........................Passed
Check for case conflicts.................................................Passed
Check that executables have shebangs.....................................Failed
- hook id: check-executables-have-shebangs
- exit code: 1

hooks/environment: marked executable but has no (or invalid) shebang!
  If it isn't supposed to be executable, try: `chmod -x hooks/environment`
  If it is supposed to be executable, double-check its shebang.

Check JSON...........................................(no files to check)Skipped
Check for merge conflicts................................................Passed
Check for broken symlinks............................(no files to check)Skipped
Detect Private Key.......................................................Failed
- hook id: detect-private-key
- exit code: 1

Private key found: python/fixture/insecure-ssl/privatekey.txt

Fix End of Files.........................................................Passed
Forbid new submodules....................................................Passed
Mixed line ending........................................................Passed
Pretty format JSON...................................(no files to check)Skipped
Sort simple YAML files...............................(no files to check)Skipped
Trim Trailing Whitespace.................................................Passed
Forbid binaries..........................................................Failed
- hook id: forbid-binary
- exit code: 1

[ERROR] python/fixture/server.zip appears to be a binary file

Test shell scripts with shellcheck.......................................Failed
- hook id: shellcheck
- exit code: 1

In hooks/checkout line 31:
  trap "rm -rf ${temp_venv_dir}" EXIT
               ^--------------^ SC2064: Use single quotes, otherwise this expands now rather than when signalled.


In hooks/checkout line 33:
  ${temp_venv_dir}${venv_python_bin} -m pip install -r "${plugin_root}/python/requirements.txt"
  ^--------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
  "${temp_venv_dir}"${venv_python_bin} -m pip install -r "${plugin_root}/python/requirements.txt"


In hooks/checkout line 40:
${venv_dir}${venv_python_bin} "${plugin_root}/python/checkout.py"
^---------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
"${venv_dir}"${venv_python_bin} "${plugin_root}/python/checkout.py"

For more information:
  https://www.shellcheck.net/wiki/SC2064 -- Use single quotes, otherwise this...
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

In hooks/pre-checkout line 20:
    SANITIZED_STREAM=$(echo $STREAM | python -c "import sys; print(sys.stdin.read().split('/')[2]);")
                            ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
    SANITIZED_STREAM=$(echo "$STREAM" | python -c "import sys; print(sys.stdin.read().split('/')[2]);")


In hooks/pre-checkout line 23:
    SANITIZED_STREAM=$(echo $STREAM | python -c "import sys; print(sys.stdin.read().replace('/', '_'));")
                            ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
    SANITIZED_STREAM=$(echo "$STREAM" | python -c "import sys; print(sys.stdin.read().replace('/', '_'));")

For more information:
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

Check shell style with shfmt.............................................Passed
pylint...................................................................Failed
- hook id: pylint
- exit code: 30

************* Module buildkite-trigger
examples/buildkite-trigger.py:3:0: C0301: Line too long (118/100) (line-too-long)
examples/buildkite-trigger.py:1:0: C0103: Module name "buildkite-trigger" doesn't conform to snake_case naming style (invalid-name)
examples/buildkite-trigger.py:1:0: C0114: Missing module docstring (missing-module-docstring)
************* Module buildkite
python/buildkite.py:103:2: W0511: TODO: Remove this to discourage git-based pipelines that sync perforce (fixme)
python/buildkite.py:123:0: C0301: Line too long (119/100) (line-too-long)
python/buildkite.py:72:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
python/buildkite.py:84:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
python/buildkite.py:100:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
python/buildkite.py:7:0: W0611: Unused import re (unused-import)
python/buildkite.py:8:0: W0611: Unused datetime imported from datetime (unused-import)
************* Module test_perforce
python/test_perforce.py:56:0: C0301: Line too long (108/100) (line-too-long)
python/test_perforce.py:107:0: C0301: Line too long (107/100) (line-too-long)
python/test_perforce.py:332:0: C0301: Line too long (102/100) (line-too-long)
python/test_perforce.py:350:0: C0301: Line too long (117/100) (line-too-long)
python/test_perforce.py:14:0: E0401: Unable to import 'pytest' (import-error)
python/test_perforce.py:45:4: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:85:62: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:91:4: C0103: Variable name "serverRoot" doesn't conform to snake_case naming style (invalid-name)
python/test_perforce.py:101:32: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:213:14: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:213:22: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:213:14: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:233:18: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:233:26: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:233:18: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:255:31: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:255:39: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:255:31: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:262:30: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:262:38: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:262:30: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:269:35: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:269:43: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:269:35: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:276:25: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:276:33: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:276:25: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:288:24: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:288:32: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:288:24: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:300:25: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:300:33: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:300:25: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:307:28: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:307:36: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:307:28: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:314:28: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:314:36: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:314:28: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:327:28: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:327:36: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:327:28: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:356:26: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:356:34: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:380:15: W0212: Access to a protected member _read_patched of a client class (protected-access)
python/test_perforce.py:356:26: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:404:8: C0103: Variable name "s" doesn't conform to snake_case naming style (invalid-name)
python/test_perforce.py:405:8: C0103: Variable name "d" doesn't conform to snake_case naming style (invalid-name)
python/test_perforce.py:412:26: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:412:34: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:412:26: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:430:26: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:430:34: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:430:26: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:454:36: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:454:44: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:454:36: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:486:26: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:486:34: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:486:26: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:493:25: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:493:33: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:493:25: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:505:29: W0621: Redefining name 'server' from outer scope (line 72) (redefined-outer-name)
python/test_perforce.py:505:37: W0621: Redefining name 'tmpdir' from outer scope (line 83) (redefined-outer-name)
python/test_perforce.py:505:29: W0613: Unused argument 'server' (unused-argument)
python/test_perforce.py:4:0: W0611: Unused contextmanager imported from contextlib (unused-import)
************* Module cleanup-unused-workspaces
examples/cleanup-unused-workspaces.py:1:0: C0103: Module name "cleanup-unused-workspaces" doesn't conform to snake_case naming style (invalid-name)
examples/cleanup-unused-workspaces.py:1:0: C0114: Missing module docstring (missing-module-docstring)
examples/cleanup-unused-workspaces.py:5:0: E0401: Unable to import 'P4' (import-error)
examples/cleanup-unused-workspaces.py:54:8: W0702: No exception type(s) specified (bare-except)
examples/cleanup-unused-workspaces.py:6:0: C0411: standard import "from datetime import datetime, timedelta" should be placed before "from P4 import P4" (wrong-import-order)
examples/cleanup-unused-workspaces.py:7:0: C0411: standard import "from pprint import pprint" should be placed before "from P4 import P4" (wrong-import-order)
************* Module checkout
python/checkout.py:5:0: W0611: Unused import argparse (unused-import)
python/checkout.py:6:0: W0611: Unused import subprocess (unused-import)
************* Module perforce
python/perforce.py:203:2: W0511: TODO: Add a fast implementation of p4 clean here (fixme)
python/perforce.py:56:0: C0301: Line too long (105/100) (line-too-long)
python/perforce.py:73:0: C0301: Line too long (112/100) (line-too-long)
python/perforce.py:164:0: C0301: Line too long (102/100) (line-too-long)
python/perforce.py:17:0: R0902: Too many instance attributes (12/7) (too-many-instance-attributes)
python/perforce.py:20:4: R0913: Too many arguments (9/5) (too-many-arguments)
python/perforce.py:109:40: W0212: Access to a protected member _stream of a client class (protected-access)
python/perforce.py:113:19: W0212: Access to a protected member _stream of a client class (protected-access)
python/perforce.py:115:12: W0212: Access to a protected member _stream of a client class (protected-access)
python/perforce.py:115:37: W0212: Access to a protected member _stream of a client class (protected-access)
python/perforce.py:122:12: W0212: Access to a protected member _stream of a client class (protected-access)
python/perforce.py:276:4: C0116: Missing function or method docstring (missing-function-docstring)
python/perforce.py:285:8: C0415: Import outside toplevel (concurrent.futures.ThreadPoolExecutor) (import-outside-toplevel)
python/perforce.py:334:25: W0621: Redefining name 'stat' from outer scope (line 9) (redefined-outer-name)
python/perforce.py:334:4: C0103: Method name "outputStat" doesn't conform to snake_case naming style (invalid-name)
python/perforce.py:334:4: C0116: Missing function or method docstring (missing-function-docstring)
python/perforce.py:326:0: R0903: Too few public methods (1/2) (too-few-public-methods)
python/perforce.py:1:0: R0801: Similar lines in 2 files
==cleanup-unused-workspaces:12
==perforce:57
logger = logging.getLogger("p4python")
logger.setLevel(logging.INFO)
handler = logging.StreamHandler(sys.stdout)
formatter = logging.Formatter(
    '%(asctime)s %(name)s %(levelname)s: %(message)s',
    '%H:%M:%S',
)
handler.setFormatter(formatter)
logger.addHandler(handler) (duplicate-code)

------------------------------------------------------------------
Your code has been rated at 8.08/10 (previous run: 8.21/10, -0.13)

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

examples/buildkite-trigger.py:3:91: E501 line too long (118 > 90 characters)
python/buildkite.py:7:1: F401 're' imported but unused
python/buildkite.py:8:1: F401 'datetime.datetime' imported but unused
python/buildkite.py:123:91: E501 line too long (119 > 90 characters)
python/buildkite.py:140:91: E501 line too long (96 > 90 characters)
python/test_perforce.py:4:1: F401 'contextlib.contextmanager' imported but unused
python/test_perforce.py:56:91: E501 line too long (108 > 90 characters)
python/test_perforce.py:107:91: E501 line too long (107 > 90 characters)
python/test_perforce.py:332:91: E501 line too long (102 > 90 characters)
python/test_perforce.py:350:91: E501 line too long (117 > 90 characters)
python/test_perforce.py:396:91: E501 line too long (95 > 90 characters)
python/test_perforce.py:481:91: E501 line too long (97 > 90 characters)
python/test_perforce.py:522:91: E501 line too long (96 > 90 characters)
examples/cleanup-unused-workspaces.py:54:9: E722 do not use bare 'except'
python/checkout.py:5:1: F401 'argparse' imported but unused
python/checkout.py:6:1: F401 'subprocess' imported but unused
python/perforce.py:56:91: E501 line too long (105 > 90 characters)
python/perforce.py:73:91: E501 line too long (112 > 90 characters)
python/perforce.py:112:91: E501 line too long (100 > 90 characters)
python/perforce.py:131:91: E501 line too long (93 > 90 characters)
python/perforce.py:164:91: E501 line too long (102 > 90 characters)
python/perforce.py:341:91: E501 line too long (95 > 90 characters)

black....................................................................Passed

I'm comfortable with that set now becoming incremental-linting fixes, though before doing that I'd extend the readme to explain the workflow. WDYT?

petemounce self-assigned this Oct 20, 2020

petemounce requested a review from ca-johnson October 20, 2020 22:32

ca-johnson reviewed Oct 21, 2020

View reviewed changes

dev/setup_env_osx.sh Outdated Show resolved Hide resolved

ca-johnson mentioned this pull request Oct 21, 2020

Workflow improvements #194

Merged

petemounce force-pushed the workflow-autofixes branch from f8ce19d to 4792c48 Compare October 21, 2020 15:02

petemounce marked this pull request as ready for review October 21, 2020 16:09

petemounce force-pushed the workflow-autofixes branch 2 times, most recently from 5dbfdb6 to 913a94a Compare October 22, 2020 12:37

Peter Mounce added 2 commits October 27, 2020 09:25

Apply automatic fixes for python

d7b5eff

more lenient line length, we have big monitors

7254592

petemounce force-pushed the workflow-autofixes branch from 6663658 to 7254592 Compare October 27, 2020 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Workflow autofixes #195

Workflow autofixes #195

Uh oh!

petemounce commented Oct 20, 2020 •

edited

Loading

Uh oh!

improbable-prow-robot commented Oct 20, 2020

Uh oh!

ca-johnson left a comment •

edited

Loading

Uh oh!

Uh oh!

ca-johnson commented Oct 21, 2020

Uh oh!

petemounce commented Oct 22, 2020

Uh oh!

Uh oh!

Workflow autofixes #195

Are you sure you want to change the base?

Workflow autofixes #195

Uh oh!

Conversation

petemounce commented Oct 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Verification

Uh oh!

improbable-prow-robot commented Oct 20, 2020

Uh oh!

ca-johnson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ca-johnson commented Oct 21, 2020

Uh oh!

petemounce commented Oct 22, 2020

Uh oh!

Uh oh!

petemounce commented Oct 20, 2020 •

edited

Loading

ca-johnson left a comment •

edited

Loading