Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.7] bpo-35283: Add a deprecated warning for the threading.Thread.isAlive #11459

Closed
wants to merge 1 commit into from

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Jan 7, 2019

@corona10
Copy link
Member Author

corona10 commented Jan 7, 2019

@asvetlov @vstinner
Please take a look, this PR is a backport version of #11454
Thanks!

Lib/threading.py Outdated Show resolved Hide resolved
Lib/threading.py Outdated Show resolved Hide resolved
Lib/threading.py Outdated Show resolved Hide resolved
Lib/threading.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@vstinner
Copy link
Member

vstinner commented Jan 7, 2019

In practice, I would prefer to start with a change in the master branch, and later consider what to do in the 3.7 branch. I'm not sure if it's ok to start to emit a PendingDeprecatinonWarning in 3.7.3.

@corona10 corona10 changed the title [3.7] bpo-35283: Add a _DummyThread.isAlive alias and a deprecated msg. [3.7] bpo-35283: Add a deprecated warning for the threading.Thread.isAlive Jan 7, 2019
@corona10
Copy link
Member Author

corona10 commented Jan 7, 2019

I have made the requested changes; please review again.
Also, I reflect your review on the master branch PR #11454
thanks!

@bedevere-bot
Copy link

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

@eamanu
Copy link
Contributor

eamanu commented Jan 7, 2019

I have made the requested changes; please review again.
Also, I reflect your review on the master branch PR #11454
thanks!

IMO you have to close this PR, and then when the change is accepted on master branch (#11454), will be backport to 3.x

@corona10
Copy link
Member Author

corona10 commented Jan 8, 2019

@eamanu

Most of the case, the backport patch you mentioned can be written by a bot.
(e.g #7204) So, we don't have to open seperate PR.

But in this case, as vstiner mentioned, we should apply PendingDeprecationWarning for python 3.7.
So for to do this, IMHO manual backport patch is a right way. if not
python3.7 will have a weird change to isAlive() method with deprecating message.
DeprecationWarning -> PendingDeprecationWarning

So I think that this PR can be merged after #11454 is merged when core developers decide to emit PendingDeprecationWarning to 3.7 also .

@eamanu
Copy link
Contributor

eamanu commented Jan 8, 2019

@eamanu

Most of the case, the backport patch you mentioned can be written by a bot.
(e.g #7204) So, we don't have to open seperate PR.

But in this case, as vstiner mentioned, we should apply PendingDeprecationWarning for python 3.7.
So for to do this, IMHO manual backport patch is a right way. if not
python3.7 will have a weird change to isAlive() method with deprecating message.
DeprecationWarning -> PendingDeprecationWarning

So I think that this PR can be merged after #11454 is merged when core developers decide to emit PendingDeprecationWarning to 3.7 also .

Yes you right. But the when the bot generate the backport, maintainer could modify the PR, isn't?

@corona10
Copy link
Member Author

corona10 commented Jan 8, 2019

@eamanu
Yeah.. but that will also need manually updating for the commit.
So sometimes in my experience, I wrote a manual backport patch for other python versions.
ex) #8156

So do I have to close this PR?

@corona10
Copy link
Member Author

corona10 commented Jan 8, 2019

@vstinner
But as @eamanu mentioned,
I think that we can re-create backport patch after #11454 is merged.
I will close this PR.

@corona10 corona10 closed this Jan 8, 2019
@corona10 corona10 deleted the bpo-35283-3.7 branch August 12, 2019 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants