-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-125665: Update turtledemo docstrings with correct file names #125691
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
Conversation
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 |
As explained in #118673 (comment), backports will have merge conflicts, like #123457 did. So fixing more than the 3 header lines now is likely a good idea. I believe that if you have the diff in a file, you can just add EDIT: Cancel this: "We would have to check all examples in 3.13 to make sure they have the same exact |
I decided to skip news as there is no reason I can think of that anyone should be hinted to possibly read the revised docstring. |
Sorry I didn't understand what should I do :( . Do I need to open another PR of 3.13/3.12 to resolve the conflict after merge? The above suggestion has been resolved, thanks edit: I have made the requested changes; please review again |
I unresolved comments 1 and 5 (though gh falsely still claims outdated). Fix 5 first. I believe 1 can be done with sed, which comes with Git for Windows. Start menu => Git => Git Bash starts a Linux-like terminal that recognizes sed (stream editor) commands. I used it several years ago after finding and reading a sed manual. I will do so if need be. (It might be easier to first undo the change to (edit) colormixer.py in the local branch version -- I don't know if sed on multiple files stops on error or merely reports and continues with the next one.) When I merge to main, I will try backporting to be sure that the merge conflict for clock.py was due to (or remains after) changing the docstring rather than only resulting from deleting the coding line. Assuming the backport fails, the first step will be to switch to a 3.13 branch (or workspace) and copy the new main turtledemo files into the turtledemo directory of a 3.13 PR branch. I may ask other core developers if we need to add back the #! line. If so, sed would be easiest. I will let you try if you wish or refresh my sed-fu. |
Looks good as of my last comments. Tomorrow I will recheck. Since there is likely to be a backport problem, I will also expand comments to see if I want to make any further changes now before merging. |
I added periods to 3 files. Any further changes past line 3 will backport properly. |
Thanks @Wulian233 for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @Wulian233 and @terryjreedy, I could not cleanly backport this to
|
Of the 19 examples, 13 have shebang lines an have the executable mode bit; the other 6 have neither. Using cherry_picker showed that the 13 have merge conflicts because of the shebang lines. I think we should leave shebang line presence and file mode alone. I am thinking about how to be best do this while changing the docstring. |
Thank you for the main PR. I can think of 3 backport approaches, and all seem more trouble than the result is worth. I am now inclined to skip backports, close the issue, and move on to other isssues. |
If you'd like, I can create a manual target of 3.13 Should I keep or remove shebang? Retaining shebang will still be a conflict in future backports, will removing shebang be considered a new feature and not accept 3.13 backport? |
At least part of the reason that shebang/mode changes were not backported was compatibility considerations (https://peps.python.org/pep-0387/). (IDLE exception. ) As I said above, "we should leave shebang line presence and file mode alone". This multiplies the time needed for a [3.13] backport by at least 2. If you produce one ready-to-merge for fun/instruction, I will merge it, but I prefer to work on other issues. |
…python#125691) Co-authored-by: Wulian <xiguawulian@gmail.com> Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
edit: skip news I think
📚 Documentation preview 📚: https://cpython-previews--125691.org.readthedocs.build/