Skip to content

Refactor PyImport_ImportModuleLevelObject(). #4680

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

Merged
merged 2 commits into from
Dec 3, 2017

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Dec 2, 2017

Add import_find_and_load() helper function. The addition of
the importtime option has made PyImport_ImportModuleLevelObject() large
and so using a helper seems worthwhile. It also makes it clearer that
abs_name is the only argument needed by _find_and_load().

Add import_find_and_load() helper function.  The addition of
the importtime option has made PyImport_ImportModuleLevelObject() large
and so using a helper seems worthwhile.  It also makes it clearer that
abs_name is the only argument needed by _find_and_load().
@nascheme nascheme force-pushed the import_find_and_load branch from 3e15686 to 864202a Compare December 2, 2017 21:27
Python/import.c Outdated
goto error;
}
Copy link
Member

Choose a reason for hiding this comment

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

Don't remove braces when existing code use braces.

https://www.python.org/dev/peps/pep-0007/

Code structure: one space between keywords like if, for and the following left paren; no spaces inside the paren; braces are required everywhere, even where C permits them to be omitted, but do not add them to code you are not otherwise modifying. All new C code requires braces.

Copy link
Member

@methane methane left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

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

Aside from the minor brace formatting issue @methane mentioned, this looks good to me.

@nascheme nascheme merged commit eea3cc1 into python:master Dec 3, 2017
mod != NULL);

if (import_time) {
_PyTime_t cum = _PyTime_GetPerfCounter() - t1;

Choose a reason for hiding this comment

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

I see .. a cultured individual .. good job 🔥

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.

6 participants