Opened 6 years ago

Closed 6 years ago

#7802 enhancement closed fixed (fixed)

minor implementation clarity improvement for readBody warning

Reported by: Glyph Owned by: Glyph
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/readBody-None-transport-7802
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description

It's not quite enough to warrant a revert, but exarkun's comments on #7762 should nevertheless be addressed.

Change History (6)

comment:1 Changed 6 years ago by Glyph

Author: glyph
Branch: branches/readBody-None-transport-7802

(In [43974]) Branching to readBody-None-transport-7802.

comment:2 Changed 6 years ago by Glyph

Keywords: review added

comment:3 Changed 6 years ago by Adi Roiban

Keywords: review removed
Owner: set to Glyph

Changes looks good for merge.

are there any guidelines for where to place inline function definitions, ex place them at the start of method parent method.

Thanks!

comment:4 in reply to:  3 Changed 6 years ago by Glyph

Replying to adiroiban:

Changes looks good for merge.

are there any guidelines for where to place inline function definitions, ex place them at the start of method parent method.

Nope. This is a matter of taste for the author :-).

Generally, I like to define callbacks for Deferreds after the Deferred themselves, so that the code flow is in order of temporal execution. In the case of cancellers, you have to declare them first, which in this case is unfortunate since the canceller references the protocol which has to be declared after the Deferred is created, since there's a circular reference. If I could have defined things in a different order here I probably would have.

Thanks!

Thanks for the review!

comment:5 Changed 6 years ago by Glyph

Status: newassigned

comment:6 Changed 6 years ago by Glyph

Resolution: fixed
Status: assignedclosed

(In [43995]) Merge readBody-None-transport-7802: fix a brief issue.

Author: glyph

Reviewer: adiroiban

Fixes: #7802

Use 'is' to compare the transport, avoid doing the same "getattr" twice in different ways.

Note: See TracTickets for help on using tickets.