Opened 10 years ago
Closed 10 years ago
#6025 enhancement closed fixed (fixed)
Port twisted.protocols.basic to Python 3
Reported by: | Itamar Turner-Trauring | Owned by: | Itamar Turner-Trauring |
---|---|---|---|
Priority: | normal | Milestone: | Python 3.3 Minimal |
Component: | core | Keywords: | |
Cc: | Branch: |
branches/basic-py3-6025-3
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | itamarst |
Description
twisted.protocols.basic
should run on Python 3.
Change History (7)
comment:1 Changed 10 years ago by
Author: | → itamarst |
---|---|
Branch: | → branches/basic-py3-6025 |
comment:2 Changed 10 years ago by
Branch: | branches/basic-py3-6025 → branches/basic-py3-6025-2 |
---|---|
Keywords: | review added |
Owner: | set to Jean-Paul Calderone |
Ugh, for some reason the branch wasn't updated. Build running here:
http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/basic-py3-6025-2
comment:3 Changed 10 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Itamar Turner-Trauring |
Thanks. Looks like you had some fun on this one.
Ugh, for some reason the branch wasn't updated. Build running here:
When this happens, please follow the directions.
- The
test-case-name
ontwisted/protocols/basic.py
needs to be updated - The byte literals in the documentation for NetstringReceiver._LENGTH should be in C{}
- Lots of tests in the newly created
twisted.protocols.test.test_basic
don't follow the coding standard (misnamed, poorly written docstrings, usefail
variation of assertion methods, etc). Please file a ticket for fixing them. - I guess my general thoughts regarding
_PY
is that it should be used at top-level module scope and not anywhere else. Particularly the cases which aren't going to go away, like cases that deal with the differences between Python 2 str and Python 3 bytes, as inLineOnlyReceiverTestCase.test_buffer
. Using it to define two alternate implementations of a helper function seems tolerable (and a function for getting an iterator over bytes that yields each individual byte as a bytes object instead of an integer sounds like one that will be used more than once anyway - newtwisted.python.compat
API?), but constantly re-checking it after import time bothers me, both for performance reasons and because of the tedium it introduces for debugging and overall cognitive load of a section of code. This approach would avoid worsening the Python 2 performance of the implementation ofsendString
, too.- Other tests in this module that could use such a helper:
- LPTestCaseMixin.test_illegal
- NetstringReceiverTestCase.test_receiveNetstringPortions_3
- IntNTestCaseMixin.test_receive
- IntNTestCaseMixin.test_partial
- Other tests in this module that could use such a helper:
- Something about new-style classes...
NewStyleInt16TestCase
is redundant on Python 3 I guess. Can we indicate that somehow, so eventually it's clear we can delete it? - Sorry, I think the new comments are largely inappropriate - at least, as comments in the source. They would probably make a good labs blog post though.
comment:4 Changed 10 years ago by
Branch: | branches/basic-py3-6025-2 → branches/basic-py3-6025-3 |
---|
(In [35916]) Branching to 'basic-py3-6025-3'
comment:5 Changed 10 years ago by
Keywords: | review added |
---|---|
Owner: | changed from Itamar Turner-Trauring to Jean-Paul Calderone |
Addressed all review comments. I skipped the newstyle tests, added utility method for netstring formatting, and sanitized the comment about Python 3.
Started http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/basic-py3-6025-3
comment:6 Changed 10 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Itamar Turner-Trauring |
- I guess there should be a .removal news fragment, for the removal of writing non-str.
LPTestCaseMixin.test_illegal
still has a subjective comment about Python 3, and some code that would be simplified byiterbytes
.
That's all. Please merge once those are addressed.
comment:7 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [35830]) Branching to 'basic-py3-6025'