Fade to black #314

wsanchez · 2019-12-10T23:17:11Z

This PR will adopt black code formatting for Klein.

Note that I have not checked in any of the reformatted files yet, because if I do, merging this after some other PR gets merged will be nutso. So this is currently just checking to see if people agree with this configuration change.

To see what the code changes look like, check out this branch and run:

tox -e black-reformat

At that point the flake8 and black Tox environments should pass.

Move environment config nearer to where it is used.

wsanchez · 2019-12-11T00:25:52Z

.travis.yml

@@ -102,8 +104,8 @@ matrix:


 install:
-  - ./.travis/install


This script doesn't do anything useful any more; tossing it.

wsanchez · 2019-12-11T00:25:57Z

.travis.yml



 script:
-  - ./.travis/run tox


This script doesn't do anything useful any more; tossing it.

wsanchez · 2019-12-11T00:28:12Z

@glyph @twm @hawkowl @moshez thoughts?

wsanchez · 2019-12-11T00:29:12Z

src/klein/test/test_resource.py

+                (
+                    "Request.finish called on a request after its connection "
+                    "was lost; use Request.notifyFinish to keep track of this."
+                )
+            )


This is so that when black reformats it, we stay within max line length.

wsanchez · 2019-12-11T00:29:45Z

src/klein/test/test_memory.py

                store.newSession(True, SessionMechanism.Header)
-            )
+            ),


This is so that when black reformats it, we the # type: ignore comment for ISession stays with it.

wsanchez · 2019-12-11T00:29:59Z

src/klein/_session.py

+                        u"Can't initialize a session on a "
+                        u"{method} request.".format(
+                            method=request.method.decode("ascii")
+                        )


This is so that when black reformats it, we stay within max line length.

wsanchez · 2019-12-11T00:32:51Z

.gitignore

-/.coverage
-/.coverage.*
 /.eggs
 /.hypothesis/
-/.mypy_cache
 /.tox/
-/_trial_temp/


These scribble into ./.tox/

wsanchez · 2019-12-11T00:34:12Z

tox.ini

    docs, docs-linkcheck
+    packaging
+
+skip_missing_interpreters = {tty:True:False}


True only in interactive shells

wsanchez · 2019-12-11T00:34:53Z

tox.ini

+skip_missing_interpreters = {tty:True:False}
+
+
+[default]


This environment holds values that can be reused later, allowing use to set them in one place.

wsanchez · 2019-12-11T00:35:44Z

tox.ini


-skip_missing_interpreters = {env:TOX_SKIP_MISSING_INTERPRETERS:True}
+    PYTHONPYCACHEPREFIX={envtmpdir}/pycache


Write __pycache__ in ./tox/ to reduce the number of build artifacts left in the source tree.

wsanchez · 2019-12-11T00:36:40Z

tox.ini

-passenv =
-    # See https://github.com/codecov/codecov-python/blob/5b9d539a6a09bc84501b381b563956295478651a/README.md#using-tox
-    codecov: TOXENV
-    codecov: CI
-    codecov: TRAVIS TRAVIS_*
-


Moved down to codecov environment.

wsanchez · 2019-12-11T00:37:05Z

tox.ini

-    PIP_DISABLE_PIP_VERSION_CHECK=1
-    VIRTUALENV_NO_DOWNLOAD=1


These aren't necessary any more.

Due to tox-dev/tox#448 shipped in Tox v3.10.0. Perhaps we should set [tox] minversion = 3.10.0?

I'm open to that, but I wouldn't do that based on just these, since I think Tox still works (in most situations) if pip and virtualenv do these goofy things.

wsanchez · 2019-12-11T00:38:02Z

tox.ini


    # Run trial with coverage
-    coverage: coverage run -p "{envbindir}/trial" --random=0 --logfile="{envlogdir}/trial.log" --temp-directory="{envlogdir}/trial.d" {posargs:klein}
+    coverage: coverage run --source {env:PY_MODULE} "{envdir}/bin/trial" --random=0 --logfile="{envlogdir}/trial.log" --temp-directory="{envlogdir}/trial.d" {posargs:{env:PY_MODULE}}


Adds --source and uses {env:PY_MODULE}

wsanchez · 2019-12-11T00:39:06Z

tox.ini

+    coverage: coverage run --source {env:PY_MODULE} "{envdir}/bin/trial" --random=0 --logfile="{envlogdir}/trial.log" --temp-directory="{envlogdir}/trial.d" {posargs:{env:PY_MODULE}}
+
+    # Run coverage reports, ignore exit status
+    coverage: - coverage report --skip-covered


Notes uncovered code for this environment.

wsanchez · 2019-12-11T00:39:33Z

tox.ini

+    black {env:BLACK_LINT_ARGS:} src
+
+
+[testenv:black-reformat]


This one actually reformats the code in place.

wsanchez · 2019-12-11T00:40:57Z

tox.ini

+    ######## WARNINGS BELOW SHOULD BE FIXED ########
+
+    # Missing docstring in public module
+    D100,
+
+    # Missing docstring in public class
+    D101,
+
+    # Missing docstring in public method
+    D102,
+
+    # Missing docstring in public function
+    D103,
+
+    # Missing docstring in public package
+    D104,
+
+    # Missing docstring in magic method
+    D105,

-    # too many blank lines
-    E302,
+    # Missing docstring in __init__
+    D107,

-    # too many blank lines
-    E303,
+    # Use """triple double quotes"""
+    D300,

-    # expected 2 blank lines after class or function definition
-    E305,
+    # First word of the first line should be properly capitalized
+    D403,
+
+    # Additional newline in a group of imports
+    I202,
+
+    ######## WARNINGS ABOVE SHOULD BE FIXED ########


These are newly-enabled warnings that should be addressed in other PRs.

wsanchez · 2019-12-11T00:41:52Z

tox.ini

@@ -174,17 +258,13 @@ max-complexity = 60

 description = run Mypy (static type checker)

-basepython = python3.8
-
-skip_install = True


Don't skip installing the project (and dependencies) because that's how you get type stubs from dependencies.

wsanchez · 2019-12-11T00:43:22Z

tox.ini

+
+description = generate coverage report
+
+depends = {test,coverage}-py{27,35,36,37,38,py2,py3}-tw{171,184,current,trunk}


Wait for test environments to complete before running this one (only matters if you run them in parallel).

twm

I'm looking forward to stealing much of this for Twisted and treq!

twm · 2019-12-14T21:39:22Z

MANIFEST.in

-include LICENSE
-include README.rst
+include .coveragerc
+include .travis/twistedchecker-diff


Does it make sense to include this in distributions? It's not useful out of VCS, is it?

I think .coveragerc should be included, so you can run Tox from a source distribution and get the same results we get in development.

twistedchecker-diff should go away entirely… I'll submit a PR for that.

twm · 2019-12-14T21:51:37Z

tox.ini

-    PIP_DISABLE_PIP_VERSION_CHECK=1
-    VIRTUALENV_NO_DOWNLOAD=1


Due to tox-dev/tox#448 shipped in Tox v3.10.0. Perhaps we should set [tox] minversion = 3.10.0?

hawkowl · 2019-12-15T19:59:00Z

The travis envs fail because codecov got moved around, but otherwise, looks good. If you can fix that, run black, and verify that everything that's green in travis should be, go for it.

hawkowl · 2019-12-15T20:01:01Z

as an aside, we use this flake8 configuration on synapse:

[flake8]
# see https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes
# for error codes. The ones we ignore are:
#  W503: line break before binary operator
#  W504: line break after binary operator
#  E203: whitespace before ':' (which is contrary to pep8?)
#  E731: do not assign a lambda expression, use a def
#  E501: Line too long (black enforces this for us)
ignore=W503,W504,E203,E731,E501

I'd go with just these, tbh, and see how they go?

This reverts commit 22c1ffd.

… in python2.7, and that avoids this.

codecov · 2019-12-17T18:41:44Z

Codecov Report

❗ No coverage uploaded for pull request base (master@889ae4d). Click here to learn what that means.
The diff coverage is 98.93%.

@@            Coverage Diff            @@
##             master     #314   +/-   ##
=========================================
  Coverage          ?   98.15%           
=========================================
  Files             ?       48           
  Lines             ?     3903           
  Branches          ?      259           
=========================================
  Hits              ?     3831           
  Misses            ?       55           
  Partials          ?       17

Impacted Files	Coverage Δ
src/klein/test/_trial.py	`75% <ø> (ø)`
src/klein/_headers_compat.py	`100% <ø> (ø)`
src/klein/test/py3_test_resource.py	`74.07% <ø> (ø)`
src/klein/_iapp.py	`100% <ø> (ø)`
src/klein/_isession.py	`100% <ø> (ø)`
src/klein/__init__.py	`100% <ø> (ø)`
src/klein/_typing.py	`100% <ø> (ø)`
src/klein/_request_compat.py	`100% <ø> (ø)`
src/klein/test/test_headers_compat.py	`100% <ø> (ø)`
src/klein/storage/memory.py	`100% <ø> (ø)`
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 889ae4d...cae339e. Read the comment docs.

… running on py2.7

This reverts commit d374176.

glyph · 2019-12-18T05:28:26Z

Hey @ambv FYI

wsanchez · 2019-12-19T00:22:19Z

as an aside, we use this flake8 configuration on synapse:

[flake8]
# see https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes
# for error codes. The ones we ignore are:
#  W503: line break before binary operator
#  W504: line break after binary operator
#  E203: whitespace before ':' (which is contrary to pep8?)
#  E731: do not assign a lambda expression, use a def
#  E501: Line too long (black enforces this for us)
ignore=W503,W504,E203,E731,E501

I'd go with just these, tbh, and see how they go?

I didn't mention this, but all of the muted warnings in the config right now would cause an error if removed.

fade to black

e68dab3

wsanchez self-assigned this Dec 10, 2019

wsanchez added 6 commits December 10, 2019 15:18

Don't forget travis

cf52925

--target-version=py27

e08e9a0

target version is in pyproject.toml, actually.

654bbe5

Move environment config nearer to where it is used.

Move type comment to where black will not fiddle with it

1c84a7f

Fix packaging

6d536d6

Remove some warnings from the disabled list.

d735863