Opened 11 years ago

Closed 11 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 11 years ago by Jean-Paul Calderone

Summary: Add IWin32EventReactorAdd IReactorWin32Event

comment:2 Changed 11 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/ireactorwin32-4523

(In [29427]) Branching to 'ireactorwin32-4523'

comment:3 Changed 11 years ago by Jean-Paul Calderone

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 11 years ago by Ying Li

Keywords: review removed
Owner: set to Jean-Paul Calderone

Two minor points that may or may not be valid:

  1. 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).
  2. 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 11 years ago by Jean-Paul Calderone

  1. Rephrased the news fragment to avoid the conjunction entirely
  2. 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 11 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(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 <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.