Skip to content

bpo-40208: Remove deprecated has_exec method of SymbolTable #19396

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 13, 2020
Merged

bpo-40208: Remove deprecated has_exec method of SymbolTable #19396

merged 4 commits into from
Apr 13, 2020

Conversation

@isidentical
Copy link
Member

@isidentical isidentical commented Apr 6, 2020 •
edited by bedevere-bot
Loading

corona10
corona10 requested changes Apr 10, 2020
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update Doc/library/symtable.rst also :)
@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.
@isidentical
Copy link
Member Author

Thanks for the reminder. I have made the requested changes; please review again
@bedevere-bot
Copy link

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.
@bedevere-bot bedevere-bot requested a review from corona10 April 10, 2020 18:07
corona10
corona10 approved these changes Apr 11, 2020
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I 've also checked @gvanrossum and @terryjreedy 's comment.

It looks like safe to remove :)
Thanks for catching this!

cc @vstinner
terryjreedy
terryjreedy approved these changes Apr 11, 2020
View reviewed changes
Copy link
Member

@terryjreedy terryjreedy left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor change. Will merge after tests pass again.
Corona10 war referring to comments on issue, in particular, Guido's approval.
@terryjreedy
Copy link
Member

@corona10 Since you did initial review, go ahead and merge and close issue if you want. I will wait until my tomorrow.
@corona10
Copy link
Member

corona10 commented Apr 11, 2020 •
edited
Loading

@terryjreedy

Thanks :) Since I am in post-promotion mentoring period,
I need to ask @vstinner before pressing the merge button. (https://discuss.python.org/t/vote-to-promote-dong-hee-na/3794)
So if you don't mind, would you like to wait for 2-3 days?
but if the issue is urgent, it is okay to merge by you :)

Thank you for understanding :)
@corona10 corona10 self-assigned this Apr 12, 2020
vstinner
vstinner reviewed Apr 13, 2020
View reviewed changes
@isidentical isidentical requested a review from vstinner April 13, 2020 23:19
vstinner
vstinner approved these changes Apr 13, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @corona10: You can merge it.
@corona10 corona10 merged commit 990ea42 into python:master Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants

@isidentical @bedevere-bot @terryjreedy @corona10 @vstinner @the-knights-who-say-ni