bpo-33407: Implement Py_DEPRECATED() on Windows#8980
Conversation
ac3ad87 to
64de78b
Compare
August 28, 2018 22:23
64de78b to
72324ab
Compare
August 28, 2018 23:25
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
@serhiy-storchaka, @zooba: Would you mind to double check?
Sorry, something went wrong.
|
This breaks compatibility. |
Sorry, something went wrong.
How? Would you mind to elaborate? |
Sorry, something went wrong.
|
The current usage: extern int foo(int) Py_DEPRECATED(3.2);This PR changes it to Py_DEPRECATED(3.2) extern int foo(int);It requires to rewrite all code that uses |
Sorry, something went wrong.
|
Ok, but we have to make the change to support Visual Studio. Do you think
that the macro is used outside Python itself?
|
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
@ZackerySpytz: Can you please explain that Py_DEPRECATED() macro must now be used at the start of the function definition, rather than at the end, in a new "Changes in the C API" section at https://docs.python.org/dev/whatsnew/3.8.html#porting-to-python-3-8 ?
See https://docs.python.org/dev/whatsnew/3.7.html#changes-in-the-c-api for an example of the Python 3.7 documentation.
@ZackerySpytz: Can you please confirm that using Py_DEPRECATED() at the start of the function (rather than the end) already works in Python 3.7?
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
|
@vstinner I have added a What's New entry for this change.
Yes, I can confirm using Py_DEPRECATED() at the start of the function already worked before this change. |
Sorry, something went wrong.
|
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
Sorry, something went wrong.
|
@vstinner, I have implemented all of your requested changes, and Serhiy Storchaka has also now approved this PR. Please have a look. |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
I have requests on the documentation part.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
|
Two deprecation warnings are emitted on MSVC because of this PR; they can be seen on AppVeyor (e.g. https://ci.appveyor.com/project/python/cpython/builds/24580662#L125): I had opened https://bugs.python.org/issue36935 and GH-13355 to fix these warnings. |
Sorry, something went wrong.
|
@vstinner I have made the requested changes; please review again. |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
Sorry, something went wrong.
https://bugs.python.org/issue33407