-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Fix flake8 issue WPS336 (explicit str concat) #254
Conversation
β¦it string concat
Codecov Report
@@ Coverage Diff @@
## master #254 +/- ##
==========================================
- Coverage 78.35% 77.13% -1.22%
==========================================
Files 25 25
Lines 3816 3727 -89
==========================================
- Hits 2990 2875 -115
- Misses 826 852 +26 |
cheroot/server.py
Outdated
msg = self.server.protocol.encode('ascii') | ||
msg += b' 100 Continue\r\n\r\n' | ||
msg = b''.join([self.server.protocol.encode('ascii'), | ||
b' 100 Contunue\r\n\r\n']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b' 100 Contunue\r\n\r\n']) | |
b' 100 Continue\r\n\r\n']) |
cheroot/server.py
Outdated
msg = self.server.protocol.encode('ascii') | ||
msg += b' 100 Continue\r\n\r\n' | ||
msg = b''.join([self.server.protocol.encode('ascii'), | ||
b' 100 Contunue\r\n\r\n']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But better use string interpolation because it would be better readable this way.
Like
b'{} {}'.format(
self.server.protocol.encode('ascii'),
b' 100 Continue\r\n\r\n',
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. It is better to prefer newer and faster str.format()
over the %-interpolation syntax. Would you mind doing this?
@alephnot0 it looks like your Git is misconfigured: the commits you submitted have author email that doesn't match your GitHub account. You may want to fix that as well. |
@webknjaz no I wouldn't mind, thanks for the feedback. |
This pull request introduces 1 alert and fixes 3 when merging 1e7a23b into 85a190f - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 1 alert and fixes 3 when merging 3afec78 into 85a190f - view on LGTM.com new alerts:
fixed alerts:
|
cheroot/test/test_conn.py
Outdated
@@ -838,7 +838,8 @@ def test_Chunked_Encoding(test_client): | |||
|
|||
# Try a chunked request that exceeds server.max_request_body_size. | |||
# Note that the delimiters and trailer are included. | |||
body = b'3e3\r\n' + (b'x' * 995) + b'\r\n0\r\n\r\n' | |||
body = '{}{}{}'.format('3e3\r\n', 'x' * 995, '\r\n0\r\n\r\n') \ | |||
.encode('ascii') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use proper literals instead of heavy encode/decode methods. It's also a good place for bytes.join()
. Same with escaping EOL.
cheroot/test/test_conn.py
Outdated
@@ -971,8 +972,8 @@ def test_No_CRLF(test_client, invalid_terminator): | |||
# Initialize a persistent HTTP connection | |||
conn = test_client.get_connection() | |||
|
|||
# (b'%s' % b'') is not supported in Python 3.4, so just use + | |||
conn.send(b'GET /hello HTTP/1.1' + invalid_terminator) | |||
conn.send('GET /hello HTTP/1.1{}'.format(invalid_terminator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We work with bytes so we need to explicitly say that it's bytes.
cheroot/wsgi.py
Outdated
@@ -300,7 +300,7 @@ def get_environ(self): | |||
|
|||
# Request headers | |||
env.update( | |||
('HTTP_' + bton(k).upper().replace('-', '_'), bton(v)) | |||
('HTTP_%s' % (bton(k).upper().replace('-', '_')), bton(v)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use str.format()
in this module.
@webknjaz thank you for merging. I will be continuing to contribute as I have time. |
β What kind of change does this PR introduce?
π What is the related issue number (starting with
#
)#253 - flake8 style cleanup.
β What is the current behavior? (You can also link to an open issue here)
β What is the new behavior (if this is a feature change)?
π Other information:
π Checklist:
and description in grammatically correct, complete sentences
This change isβ