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

gh-104310: Rename the New Function in importlib.util #105255

Merged

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Jun 2, 2023

The original name wasn't as clear as it could have been. This change includes the following:

  • rename the function
  • change the default value for "disable_check" to False
  • add clues to the docstring that folks should probably not use the function

@ericsnowcurrently
Copy link
Member Author

CC @brettcannon

@ericsnowcurrently ericsnowcurrently added the needs backport to 3.12 bug and security fixes label Jun 2, 2023
@ericsnowcurrently ericsnowcurrently changed the title gh-104310: Rename the New Function in Importlib.util gh-104310: Rename the New Function in importlib.util Jun 2, 2023
ericsnowcurrently and others added 2 commits June 2, 2023 15:25
"""

def __init__(self, disable_check=True):
self.disable_check = disable_check
def __init__(self, disable_check):
Copy link
Member

Choose a reason for hiding this comment

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

Without the keyword, _incompatible_extension_module_restrictions(True) reads to me like incompatible module restrictions are turned on. I would either make the parameter keyword-only or invert the meaning (i.e., enable_check).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@bedevere-bot
Copy link

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

@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) June 8, 2023 18:15
@ericsnowcurrently ericsnowcurrently merged commit 34c63b8 into python:main Jun 8, 2023
@miss-islington
Copy link
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 8, 2023
…105255)

The original name wasn't as clear as it could have been. This change includes the following:

* rename the function
* change the default value for "disable_check" to False
* add clues to the docstring that folks should probably not use the function

---------

(cherry picked from commit 34c63b8)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
@bedevere-bot
Copy link

GH-105518 is a backport of this pull request to the 3.12 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.12 bug and security fixes label Jun 8, 2023
@ericsnowcurrently ericsnowcurrently deleted the fix-importlib-util-helper branch June 8, 2023 18:35
ericsnowcurrently added a commit that referenced this pull request Jun 8, 2023
… (gh-105518)

The original name wasn't as clear as it could have been. This change includes the following:

* rename the function
* change the default value for "disable_check" to False
* add clues to the docstring that folks should probably not use the function

---------

(cherry picked from commit 34c63b8)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
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.

5 participants