Skip to content

bpo-36601: clarify signal handler comment and remove unnecessary pid check.#12784

Merged
miss-islington merged 2 commits into
python:masterfrom
jdemeyer:bpo36601
May 10, 2019
Merged

bpo-36601: clarify signal handler comment and remove unnecessary pid check.#12784
miss-islington merged 2 commits into
python:masterfrom
jdemeyer:bpo36601

Conversation

@jdemeyer

@jdemeyer jdemeyer commented Apr 11, 2019

Copy link
Copy Markdown
Contributor

@gpshead

gpshead commented Apr 12, 2019

Copy link
Copy Markdown
Member

I suggest adding a NEWS entry that says something along the lines of "A believed now-meaningless internal check for getpid() == main_pid was removed from Python's signal handler."

@gpshead gpshead self-assigned this Apr 12, 2019
Comment thread Modules/signalmodule.c Outdated
@jdemeyer

Copy link
Copy Markdown
Contributor Author

I suggest adding a NEWS entry that says something along the lines of "A believed now-meaningless internal check for getpid() == main_pid was removed from Python's signal handler."

Done.

@jdemeyer

Copy link
Copy Markdown
Contributor Author

This was set to "awaiting merge" almost 2 weeks ago. Can this actually be merged please?

@merwok

merwok commented May 3, 2019

Copy link
Copy Markdown
Member

I find believed now-meaningless internal a little hard to parse

@gpshead

gpshead commented May 10, 2019

Copy link
Copy Markdown
Member

heh thanks, i cleaned up the news entry wording.

@gpshead gpshead changed the title bpo-36601: comment that signals can be caught by any thread bpo-36601: clarify signal handler comment and remove unnecessary pid check. May 10, 2019
@miss-islington

Copy link
Copy Markdown
Contributor

@jdemeyer: Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit d237b3f into python:master May 10, 2019
@jdemeyer
jdemeyer deleted the bpo36601 branch June 26, 2019 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants