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.

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

bpo-13553: Document tkinter.Tk args #4786

Merged
merged 8 commits into from
May 10, 2022
Merged

bpo-13553: Document tkinter.Tk args #4786

merged 8 commits into from
May 10, 2022

Conversation

csabella
Copy link
Contributor

@csabella csabella commented Dec 11, 2017 •
edited by bedevere-bot
Loading

serhiy-storchaka
serhiy-storchaka reviewed Dec 11, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
terryjreedy
terryjreedy requested changes Dec 12, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
@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.

@terryjreedy
Copy link
Member

terryjreedy commented Dec 12, 2017 •
edited
Loading

None of my comments are outdated by applying Serhiy's suggestions. My review just happened to start before the patch arrived, and except for 1, happen to be attached to the same sentences. Serhiy, please also review my comments.

serhiy-storchaka
serhiy-storchaka reviewed Dec 12, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
vstinner
vstinner reviewed Dec 12, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst
Specifies that the main window for the application is to be embedded in
the window whose identifier is given, instead of being created as an
independent toplevel window.

Copy link
Member

Choose a reason for hiding this comment

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

While we are documenting the Tk class. Would it be worth it to document Tk attributes? master, children and tk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current page reads more like a 'getting started' or 'how to', so I wasn't sure how much detail to add. I know there's a separate bug issue where Terry figured out the difference between master and parent, so maybe it would be worthwhile to include that here?

Doc/library/tkinter.rst Outdated Show resolved Hide resolved
@csabella
Copy link
Contributor Author

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

vstinner
vstinner reviewed Dec 12, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
@csabella
Copy link
Contributor Author

@terryjreedy @serhiy-storchaka @vstinner
I've made additional changes to document the attributes and to remove the documentation of readprofile. Thanks!

vstinner
vstinner reviewed Dec 15, 2017
View reviewed changes
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
Doc/library/tkinter.rst Outdated Show resolved Hide resolved
terryjreedy
terryjreedy previously approved these changes Feb 2, 2018
View reviewed changes
@terryjreedy terryjreedy closed this Feb 2, 2018
@terryjreedy terryjreedy reopened this Feb 2, 2018
@vstinner
Copy link
Member

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

@JulienPalard
Copy link
Member

@csabella Looks like this PR is almost OK, maybe just missing the mention that className permits to choose the title of the window (last message of Mark). I tried to mark as resolved every actually resolved comments, I'll let you read the rest of them a last time.

JelleZijlstra
JelleZijlstra approved these changes May 8, 2022
View reviewed changes
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.

Looks good, it seems the remaining comments don't need to be blocking.

@JelleZijlstra JelleZijlstra self-assigned this May 8, 2022
@JelleZijlstra JelleZijlstra added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels May 8, 2022
@JelleZijlstra
Copy link
Member

Planning to merge this unless someone has further comments.

@taleinat
Copy link
Contributor

taleinat commented May 9, 2022

LGTM

@JelleZijlstra JelleZijlstra merged commit c56e2bb into python:main May 10, 2022
@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 10, 2022
@csabella @miss-islington
bpo-13553: Document tkinter.Tk args (pythonGH-4786)
8d836f4 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
@bedevere-bot
Copy link

GH-92593 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed needs backport to 3.11 only security fixes needs backport to 3.10 only security fixes labels May 10, 2022
@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 10, 2022
@csabella @miss-islington
bpo-13553: Document tkinter.Tk args (pythonGH-4786)
e79711c 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 10, 2022
@bedevere-bot
Copy link

GH-92595 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 May 10, 2022
@csabella @miss-islington
bpo-13553: Document tkinter.Tk args (pythonGH-4786)
cf1f27a 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
miss-islington added a commit that referenced this pull request May 10, 2022
@miss-islington @csabella
bpo-13553: Document tkinter.Tk args (GH-4786)
74c094d 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
miss-islington added a commit that referenced this pull request May 10, 2022
@miss-islington @csabella
bpo-13553: Document tkinter.Tk args (GH-4786)
b7a8786 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
miss-islington added a commit that referenced this pull request May 10, 2022
@miss-islington @csabella
bpo-13553: Document tkinter.Tk args (GH-4786)
5ed2f11 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@miss-islington @csabella @hello-adam
bpo-13553: Document tkinter.Tk args (pythonGH-4786)
d5b0a60 
(cherry picked from commit c56e2bb)

Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>
@hauntsaninja hauntsaninja mentioned this pull request Oct 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

@vadmium vadmium vadmium left review comments

@roseman roseman roseman left review comments

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

@JulienPalard JulienPalard JulienPalard approved these changes

@taleinat taleinat taleinat approved these changes

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

@terryjreedy terryjreedy terryjreedy left review comments

Assignees

@JelleZijlstra JelleZijlstra

Labels
docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.