Opened 10 years ago
Closed 9 years ago
#5925 task closed fixed (fixed)
Deprecate twisted.python.runtime.Platform.isWinNT
Reported by: | Jean-Paul Calderone | Owned by: | Thijs Triemstra |
---|---|---|---|
Priority: | lowest | Milestone: | |
Component: | core | Keywords: | win32 |
Cc: | Thijs Triemstra | Branch: |
branches/deprecate-iswinnt-5925
branch-diff, diff-cov, branch-cov, buildbot |
Author: | thijs |
Description
Twisted only supports Windows NT-derived platforms at this point (Windows XP, Windows 7). See <http://en.wikipedia.org/wiki/Windows_NT>.
This function can only ever return true in a supported environment.
Change History (14)
comment:1 Changed 9 years ago by
Owner: | set to Thijs Triemstra |
---|---|
Status: | new → assigned |
comment:3 Changed 9 years ago by
Author: | → thijs |
---|---|
Branch: | → branches/deprecate-iswinnt-5925 |
Cc: | Thijs Triemstra added |
Keywords: | review added |
Owner: | Thijs Triemstra deleted |
Status: | assigned → new |
comment:4 follow-up: 9 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Thijs Triemstra |
- If this only returns
True
on win32, is there any reason to do anything more complicated thanreturn True
there? (In which case we can test that fact) test_isWinNT
shouldn't emit a deprecation warning.python.win32.getProgramsMenuPath
should be fixed to not useisWinNT
*before* before the deprecation ofisWinNT
(Since it currently lacks tests, this should perhaps be a separate ticket).- Consider using
SynchronousTestCase.callDeprecated
for testing the deprecation. - Consider leaving a comment that
t.p.deprecate
isn't used, to avoid a dependency on that module.
Please resubmit for review after addressing the above issues.
comment:6 Changed 9 years ago by
Consider using SynchronousTestCase.callDeprecated for testing the deprecation
Development documentation currently recommends flushWarnings
, not callDeprecated
.
comment:7 Changed 9 years ago by
Status: | new → assigned |
---|
There are also some "nt" references in twisted/python/dist.py
but that should probably be fixed in separate ticket.
comment:9 Changed 9 years ago by
Keywords: | review added |
---|---|
Owner: | Thijs Triemstra deleted |
Status: | assigned → new |
Replying to tom.prince:
- If this only returns
True
on win32, is there any reason to do anything more complicated thanreturn True
there? (In which case we can test that fact)
Replaced the logic with a simple isWindows()
check.
test_isWinNT
shouldn't emit a deprecation warning.
Fixed.
python.win32.getProgramsMenuPath
should be fixed to not useisWinNT
*before* before the deprecation ofisWinNT
(Since it currently lacks tests, this should perhaps be a separate ticket).
Fixed. Also added tests.
Additionally added a test for Platform.isKnown
and added some missing docstrings.
Forced a build.
comment:10 follow-up: 12 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Thijs Triemstra |
- It looks like there is one relevant twistedchecker error
************* Module twisted.python.test.test_win32 W9015: 68,0: Too many blank lines, expected 2
- There are a bunch of
\r
s in that file. - (optional) I think
twisted.python.win32.getProgramsMenuPath
can probably be allowed to fail on non-windows. i.e. just remove the conditional there. I guess that is an incompatible change though. assertNotNull
doesn't exists. The tests probably want to at least assert that the return value is aC{str}
, since that is what we document.
Please resubmit after addressing the above issues.
comment:12 Changed 9 years ago by
Keywords: | review added |
---|---|
Owner: | Thijs Triemstra deleted |
Replying to tom.prince:
- It looks like there is one relevant twistedchecker error
************* Module twisted.python.test.test_win32 W9015: 68,0: Too many blank lines, expected 2
Removed. Tried forcing a new build but the process just hung.. Need to get twistedchecker
installed locally.
- There are a bunch of
\r
s in that file.
Removed those. Seems these type of changes don't show up in the trac browser.
- (optional) I think
twisted.python.win32.getProgramsMenuPath
can probably be allowed to fail on non-windows. i.e. just remove the conditional there. I guess that is an incompatible change though.
I'd rather handle that in a different ticket (or deprecate these altogether).
assertNotNull
doesn't exists. The tests probably want to at least assert that the return value is aC{str}
, since that is what we document.
Used assertIsInstance
instead.
comment:13 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Thijs Triemstra |
Looks good. Please merge after the windows builders complete. (There are some unrelated failure on those builders, so check that the tests of the code modified here pass).
comment:14 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
It was only used once, in
python.win32.getProgramsMenuPath
which should be replaced onceisWinNT
is removed.