New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEP8 improvements #8

Merged
merged 2 commits into from Mar 3, 2017

Conversation

Projects
None yet
2 participants
@krysros
Contributor

krysros commented Feb 25, 2017

Fixes some code style warnings / errors generated by flake8

@@ -8,8 +8,12 @@
import six
import pytest
from cheroot._compat import HTTPConnection, HTTPSConnection, NotConnected, BadStatusLine
from cheroot._compat import ntob, urlopen
from cheroot._compat import HTTPConnection

This comment has been minimized.

@webknjaz

webknjaz Feb 25, 2017

Member

What is the motivation for this change?

This comment has been minimized.

@krysros

krysros Feb 25, 2017

Contributor
  1. Imports should usually be on separate lines.
  2. The length of the line 11 exceeds 79 characters.

This comment has been minimized.

@webknjaz

webknjaz Feb 25, 2017

Member

From the link you provided:

It's okay to say this though:

from subprocess import Popen, PIPE

so for from ... import ... it's OKay to list a few of them

they may be split into several lines using several from ... import ... statements or just multilined like

from some_lib import (
    importable1, importable2,
    importable3, importable4,
    etc,
)

This comment has been minimized.

@webknjaz

webknjaz Feb 25, 2017

Member

Regarding 79 chars limitation. Nowadays people usually set higher threshold, as we don't have 80 char terminals limitation today. Still it's a good practice to avoid long lines to make code better readable.

This comment has been minimized.

@krysros

krysros Feb 25, 2017

Contributor

I agree.

@@ -316,25 +316,29 @@ def testSlashes(self):
# Make sure GET params are preserved.

This comment has been minimized.

@webknjaz

webknjaz Feb 25, 2017

Member

Changes of this module are potentially useless, since it contains tests related to cherrypy, which are likely to be removed

This comment has been minimized.

@krysros

krysros Feb 25, 2017

Contributor

OK

accepted_queue_size=accepted_queue_size,
accepted_queue_timeout=accepted_queue_timeout)
accepted_queue_timeout=accepted_queue_timeout
)

This comment has been minimized.

@webknjaz

webknjaz Feb 25, 2017

Member

I think this paren should be one tab left, according to PEP8

krysros added some commits Feb 25, 2017

@webknjaz webknjaz merged commit a6c626d into cherrypy:master Mar 3, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

jaraco added a commit that referenced this pull request Mar 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment