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-33479: Tkinter docs #7287

Closed
wants to merge 32 commits into from
Closed

bpo-33479: Tkinter docs #7287

wants to merge 32 commits into from

Conversation

roseman
Copy link
Contributor

@roseman roseman commented May 31, 2018

bpo-33479: document threading issues in tkinter, other minor tkinter doc updates

@native-api
Copy link
Contributor

native-api commented Jun 2, 2018

I was asking to create a PR against my branch, in my fork (you can select any fork in the "base branch" when creating the PR, or go to my fork to file it).
In this PR, we won't be able to see my and your changes separately, and I won't be able to merge it.

Will still try to make a review so as not to waste more time...

Copy link
Contributor

@native-api native-api left a comment

Choose a reason for hiding this comment

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

A reference documentation's job is to fully, precisely and authoritatively describe the subject's public interface (which includes semantic) -- nothing more, nothing less.

There is thus no place in it for vague statements (because they have zero to negative informativity), weasel words, value judgements, factual inaccuracies and omitting or deliberately concealing any details critical to understanding the software's operation.

This is all the more critical that no-one on the Net, including the Python team, seems to really understand how Tkinter works, and this heavily stifles its adoption.

Likewise, reference doc has no business telling the user what to do and what not to do -- "I can decide that for myself, thank you very much!" It solely tells what is supported and what is not.
Actually advising the users how to use the software is the domain of a user's guide (that may include a tutorial).

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 Show resolved Hide resolved
Doc/library/tkinter.rst Show resolved Hide resolved
Doc/library/tkinter.rst Show resolved Hide resolved
Doc/library/tkinter.rst Show resolved Hide resolved
Doc/library/tkinter.rst Show resolved Hide resolved
@native-api
Copy link
Contributor

I agree with toning down the antagonistic comparison between Tcl/Tk and other GUI kits and between Tcl and Python.

@ned-deily
Copy link
Member

@native-api Thank you for your efforts in trying to improve Python. But please try to keep the Python Community Code of Conduct in mind while interacting here. Statements like "literally no-one on the Net knows how the damn thing works" are neither true nor helpful. In particular, @roseman has a long history of contributing to and publishing documentation about Tcl/Tk. I, for one, deeply appreciate his contributions over the years in trying to improve Python's Tkinter.

Copy link
Member

@zware zware left a comment

Choose a reason for hiding this comment

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

I haven't made it all the way through this, but have added a few thoughts.

Thank you to both @native-api and @roseman for working on this; the tkinter docs have been in want of an update for quite some time.

Doc/library/tkinter.rst Show resolved Hide resolved
Doc/library/tkinter.rst 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 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.

And if you don't make the requested changes, you will be poked with soft cushions!

Despite the "code does not rust", UI certainly does with respect to visual integration into modern applications. While an API may stay functional, if it says "gimme a Widget X" and what comes up is some ugly piece of crap that doesn't fit in with the rest of the UI, it's not meeting the programmer's expectations. I'd prefer a flashing 'warning' light, a phrase like 'virtually abandoned', etc. but I'd settle for 'older'. Something needs to be there.
@native-api
Copy link
Contributor

Merged the PR branch together with roseman#10 and roseman#8 and a little more polishing into https://github.com/native-api/cpython/tree/tkinter_docs .

@matrixise matrixise added the docs Documentation in the Doc dir label May 15, 2019
@JulienPalard
Copy link
Member

@native-api @roseman Hi! Thanks for this huge PR! Enhancing the tkinter documentation is a good thing.

How would you like to move forward on this one? It's a huge PR, with still some reviews unresolved, could you take a look at it?

@zware zware changed the title Tkinter docs bpo-33479: Tkinter docs Sep 11, 2019
@zware
Copy link
Member

zware commented Sep 11, 2019

I think at this point it may be better to close this PR and create a clean one. Also note that this is probably worth a NEWS entry, even though it's documentation. And finally, the docs builds appear to have failed on this; we'll need to make sure they pass before this can be properly reviewed.

@roseman
Copy link
Contributor Author

roseman commented Sep 11, 2019

I agree, this is a bit of a mess. Unfortunately, if there's stuff still to be resolved, I just don't have the bandwidth to help right now, at least based on how difficult the last go-around was.

@JulienPalard
Copy link
Member

@roseman To speed up the merges, the "easy" way is to do smaller PRs. If you close this one to reopen a clear one as proposed by Zachary, please open a smaller one and wait for it to be merged after going further. (Opening them in parallel may not be easy neither: you'll enconter conflicts).

@ambv
Copy link
Contributor

ambv commented Aug 10, 2021

@roseman, hi Mark, if you're still willing to get this landed into Python docs, I can facilitate that. To do that, we ideally need this split into 3 - 4 smaller PRs that can be more easily merged. If you don't have time to do that, at least refreshing this PR so it's both mergeable (there are conflicts now with current docs) and up to date would be required. Thanks!

@roseman
Copy link
Contributor Author

roseman commented Aug 10, 2021

Thanks, I would appreciate it! It'll take me several days at least to get to this and go through all the previous arguments and come up with a coherent revision. Will split it up into several new PR's.

@ambv
Copy link
Contributor

ambv commented Aug 10, 2021

Take your time, @roseman, and tag me in your new PRs, so I prioritize them. Good luck! 🤞🏻🤞🏻

@ambv
Copy link
Contributor

ambv commented Aug 11, 2021

GH-27717 merged!

@ambv
Copy link
Contributor

ambv commented Aug 13, 2021

Anything else you'd like to extract from this PR, Mark?

@roseman
Copy link
Contributor Author

roseman commented Aug 14, 2021

no, I've got copies, so I'll close this one off and create new PR's as needed

@roseman roseman closed this Aug 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants