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

bpo-26792: Improve docstrings of runpy module run_functions #30729

Merged
merged 4 commits into from
Apr 29, 2022

Conversation

humbdrag
Copy link
Contributor

@humbdrag humbdrag commented Jan 20, 2022

@humbdrag humbdrag changed the title bpo-26792: improve docstrings of runpy module run_ functions bpo-26792: Improve docstrings of runpy module run_functions Jan 20, 2022
Lib/runpy.py Outdated Show resolved Hide resolved
@humbdrag
Copy link
Contributor Author

@kumaraditya303 Sorry for tagging, but any further thoughts?

1 similar comment
@humbdrag
Copy link
Contributor Author

@kumaraditya303 Sorry for tagging, but any further thoughts?

@kumaraditya303
Copy link
Contributor

@JelleZijlstra would you be able to review this docs change ?

@JelleZijlstra JelleZijlstra self-requested a review March 31, 2022 17:25
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks, a few suggestions

Lib/runpy.py Outdated Show resolved Hide resolved
Lib/runpy.py Outdated Show resolved Hide resolved
Lib/runpy.py Outdated Show resolved Hide resolved
Lib/runpy.py Outdated Show resolved Hide resolved
@@ -245,14 +258,19 @@ def _get_code_from_file(run_name, fname):
return code, fname

def run_path(path_name, init_globals=None, run_name=None):
Copy link
Member

Choose a reason for hiding this comment

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

hm, the docs get the name of the first param wrong. I'll create a separate PR to fix that.

@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.

@JelleZijlstra JelleZijlstra self-assigned this Apr 2, 2022
JelleZijlstra added a commit that referenced this pull request Apr 2, 2022
JelleZijlstra added a commit that referenced this pull request Apr 15, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 15, 2022
…2265)

Noticed while reviewing pythonGH-30729.
(cherry picked from commit f1e989b)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
miss-islington added a commit that referenced this pull request Apr 15, 2022
Noticed while reviewing GH-30729.
(cherry picked from commit f1e989b)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
miss-islington added a commit that referenced this pull request Apr 15, 2022
Noticed while reviewing GH-30729.
(cherry picked from commit f1e989b)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra JelleZijlstra added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Apr 26, 2022
@gvanrossum
Copy link
Member

@JelleZijlstra are you waiting for something before merging?
@kumaraditya303 do you have any more comments on this?

@JelleZijlstra
Copy link
Member

I need to make sure CI is passing first. I plan to go over all PRs assigned to me by the core sprints, so I'll make sure to get this through.

…Q1v1W.rst

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@JelleZijlstra JelleZijlstra merged commit 117836f into python:main Apr 29, 2022
@miss-islington
Copy link
Contributor

Thanks @humbdrag for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-92050 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 29, 2022
@bedevere-bot
Copy link

GH-92051 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 29, 2022
…-30729)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 117836f)

Co-authored-by: Humbled Drugman <humbled.drugman@gmail.com>
miss-islington added a commit that referenced this pull request Apr 29, 2022
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 117836f)

Co-authored-by: Humbled Drugman <humbled.drugman@gmail.com>
miss-islington added a commit that referenced this pull request Apr 29, 2022
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 117836f)

Co-authored-by: Humbled Drugman <humbled.drugman@gmail.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…2265)

Noticed while reviewing pythonGH-30729.
(cherry picked from commit f1e989b)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…-30729)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 117836f)

Co-authored-by: Humbled Drugman <humbled.drugman@gmail.com>
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.

7 participants