Skip to content

bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported objects #26677

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 2 commits into from
Jun 30, 2021
Merged

bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported objects #26677

merged 2 commits into from
Jun 30, 2021

Conversation

isidentical
Copy link
Member

@isidentical isidentical commented Jun 11, 2021 •
edited by bedevere-bot
Loading

@isidentical isidentical requested a review from markshannon June 11, 2021 17:43
@isidentical isidentical changed the title bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported ob… bpo-44313: generate LOAD_ATTR/CALL_FUNCTION for top-level imported objects Jun 11, 2021
@isidentical isidentical force-pushed the bpo-44313 branch 4 times, most recently from 86f2a4d to c7c708c Compare June 11, 2021 18:22
markshannon
markshannon requested changes Jun 14, 2021
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Great. Just two very minor issues.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@isidentical
Copy link
Member Author

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@bedevere-bot bedevere-bot requested a review from markshannon June 18, 2021 21:18
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jun 29, 2021
Merged
markshannon
markshannon approved these changes Jun 29, 2021
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Looks good now.
Sorry I missed your update, so importlib needs updating again.

@isidentical
Copy link
Member Author

Okay will regenerate them tomorrow.

@isidentical isidentical added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 30, 2021
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @isidentical for commit ab1b6c1 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 30, 2021
@markshannon
Copy link
Member

Thanks.

@markshannon markshannon merged commit 1b28187 into python:main Jun 30, 2021
sweeneyde
sweeneyde reviewed Jul 1, 2021
View reviewed changes
Lib/importlib/_bootstrap_external.py
@@ -360,6 +360,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.11a1 3455 (add MAKE_CELL bpo-43693)
# Python 3.11a1 3456 (interleave cell args bpo-43693)
# Python 3.11a1 3457 (Change localsplus to a bytes object bpo-43693)
# Python 3.11a1 3458 (imported objects now don't use LOAD_METHOD/CALL_METHOD)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the actual number be bumped below? I don't think it matters much in this case, but for the sake of consistency/clarity for the next bump, it might be good to fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, probably got away during rebase. Will fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sweeneyde sweeneyde sweeneyde left review comments

@markshannon markshannon markshannon approved these changes

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@isidentical @bedevere-bot @markshannon @Fidget-Spinner @sweeneyde @the-knights-who-say-ni