Remove considerations for Windows 2000 and Windows Vista#2667
Conversation
Avasam
force-pushed
the
Remove-considerations-for-Windows-2000-and-Windows-Vista
branch
from
2850ae8 to
b64507f
Compare
Avasam
force-pushed
the
Remove-considerations-for-Windows-2000-and-Windows-Vista
branch
from
b64507f to
e1d0f07
Compare
Avasam
force-pushed
the
Remove-considerations-for-Windows-2000-and-Windows-Vista
branch
from
e1d0f07 to
3c3a6e9
Compare
| #define CHECK_PFN(fname) \ | ||
| if (pfn##fname == NULL) \ | ||
| return PyErr_Format(PyExc_NotImplementedError, "%s is not available on this platform", #fname); | ||
| // Not available on Vista or earlier |
There was a problem hiding this comment.
I don't hate these comments because they help explain why there is a function pointer used. Removing the comment removes all context. I'm fine with updating the comment to be current and supply the context, or to remove the function pointer entirely.
There was a problem hiding this comment.
I'll add back or modify the comment.
Edit: this file only contained a single CHECK_PFN usage, removed along with comment.
Anyway I realized it later, but I think all these CHECK_PFN macros can be removed (looks like they're all for functions that should now always exist at build and runtime).
But I'd do that in its own PR. The main concern there will be to not accidentally remove cygwin workaround/support.
There was a problem hiding this comment.
Anyway I realized it later, but I think all these
CHECK_PFNmacros can be removed (looks like they're all for functions that should now always exist at build and runtime).
Right - I was trying to suggest that this means we can remove the function pointers entirely rather than just the null checks.
There was a problem hiding this comment.
Right - I was trying to suggest that this means we can remove the function pointers entirely rather than just the null checks.
that's what I ended up doing. Which rendered the line CHECK_PFN(SHGetPropertyStoreForWindow) dead code, so I removed it. Making the macro CHECK_PFN (in this file) unused, so I removed it.
| // interface. | ||
| static PyObject *PyAssocCreateForClasses(PyObject *self, PyObject *args) | ||
| { | ||
| // @comm This function is only available on Vista and later; a |
There was a problem hiding this comment.
kinda like the above, I think I'd rather just avoid using the function pointer entirely (and ditto below)
| Microsoft released a patch for handling time zones in 2007 (see | ||
| https://learn.microsoft.com/en-us/troubleshoot/windows-client/system-management-components/daylight-saving-time-help-support) | ||
|
|
||
| As a result, patched systems will give an incorrect result for |
There was a problem hiding this comment.
"will" seems more correct than "would" here. I get that maybe you are trying to refer to the past, but if any of those systems are still running and are patched, they will do what's described, right?
There was a problem hiding this comment.
but if any of those systems are still running and are patched, they will do what's described, right?
Correct, although any of those systems can't run pywin32 anymore (all systems that can run pywin32 are vista-or-later). I kept the tests for coverage sake.
In french we have a verb tense specifically for this scenario ^^" (conditional past).
Anyway I don't mind changing back to will rather than would/would have.
|
@mhammond I think all review comments are addressed here. Concerning other |
…e-considerations-for-Windows-2000-and-Windows-Vista
mhammond
left a comment
mhammond
left a comment
There was a problem hiding this comment.
but that said, this seems like an improvement, thanks.
| typedef HRESULT(WINAPI *PFNAssocCreate)(CLSID, REFIID, LPVOID); | ||
| static PFNAssocCreate pfnAssocCreate = NULL; | ||
|
|
||
| typedef HRESULT(WINAPI *PFNAssocCreateForClasses)(const ASSOCIATIONELEMENT *, ULONG cClasses, REFIID riid, void **ppv); |
There was a problem hiding this comment.
I don't understand why you removed the pfn for only some? What makes them special that any other considerations don't apply to them?
Avasam
deleted the
Remove-considerations-for-Windows-2000-and-Windows-Vista
branch
2 participants
Mostly updating obsolete documentation and simplifying some tests. I searched for the terms "Windows 2000" and "Vista".
Along with #2668, this removes all usages of
sys.getwindowsversion(), pushing the Python code to be more Windows-version agnostic.Most of the actual code changes had already been done in:
win32com.shell.shell: Drop support for Vista, set Windows 7 as the minimal Windows version. #2487