Opened 12 years ago
Closed 12 years ago
#4523 enhancement closed fixed (fixed)
Add IReactorWin32Event
Reported by: | Jean-Paul Calderone | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | core | Keywords: | |
Cc: | Branch: |
branches/ireactorwin32-4523
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | exarkun |
Description
win32eventreactor
has addEvent
and removeEvent
methods which are generally useful for Windows-specific stuff, plus useful in the Windows implementation of cross-platform APIs such as twisted.internet.serialport
. It would be great if these methods were part of a documented, declared interface (eg, #3802 wants to inspect the reactor for this to provide better failure messages when serialport is used with the wrong reactor).
Change History (7)
comment:1 Changed 12 years ago by
Summary: | Add IWin32EventReactor → Add IReactorWin32Event |
---|
comment:2 Changed 12 years ago by
Author: | → exarkun |
---|---|
Branch: | → branches/ireactorwin32-4523 |
comment:3 Changed 12 years ago by
Keywords: | review added |
---|---|
Owner: | Jean-Paul Calderone deleted |
I pulled the implementation of this out of the #3802 branch. Very short and simple, please review. build results
comment:4 Changed 12 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Jean-Paul Calderone |
Two minor points that may or may not be valid:
- The last half of the news fragment is awkwardly phrased ("and twisted.internet.win32eventreactor declares that it implements it.") - the two its are confusing, since each one refers to a different antecedent. Perhaps change the last "it" to "IReactorWin32Events", or change the whole thing "twisted.internet.interfaces.IReactorWin32Events is now explicitly defined and is declared to be implemented by twisted.internet.win32eventreactor" (although passive voice may not be better).
- Should test_win32events be a declared test case for win32eventreactor? (Should "-*- test-case-name: twisted.internet.test.test_win32events -*-" be at the top of the file?)
Other than that, looks good to merge.
comment:5 Changed 12 years ago by
- Rephrased the news fragment to avoid the conjunction entirely
- Since tons of other tests still apply, I don't think there's a reason to have a test-case-name on win32eventreactor.py
comment:6 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [29477]) Merge ireactorwin32-4523
Author: ltaylor.volks, exarkun Reviewer: cyli Fixes: #4523
Introduce a new interface for the Windows event support win32eventreactor already had. Also add a simple test for the existing implementation and make the reactor declare that the interface is indeed implemented.
comment:7 Changed 11 years ago by
Owner: | Jean-Paul Calderone deleted |
---|
(In [29427]) Branching to 'ireactorwin32-4523'