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-30826: Improve control flow examples #15407

Merged
merged 1 commit into from
Aug 23, 2019

Conversation

rhettinger
Copy link
Contributor

@rhettinger rhettinger commented Aug 23, 2019

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

An improvement worth merging as is, with suggestions for maybe making it even better.

Doc/tutorial/controlflow.rst Show resolved Hide resolved
inserting ``defenestrate`` over and over again.
Code that modifies a collection while iterating over that same collection can
be tricky to get right. Instead, it is usually more straight-forward to loop
over a copy of the collection or to create a new collection::
Copy link
Member

Choose a reason for hiding this comment

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

The three lines are clearer and more correct in giving both strategies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're happy with the proposed wording, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. There is no checkbox for "(*) Approve either as is as a definite and acceptable improvement over the status quo or after possible further improvements based on my comments -- you decide."

In this case, you considered each suggestion and and gave a short coherent reason why not, one which I can consider when writing or reviewing other doc patches.

I appreciate you tackling some of these lingering doc issues which have seemingly been blocked by the quest for perfection even when improvement is clearly possible.

Doc/tutorial/controlflow.rst Show resolved Hide resolved
Doc/tutorial/controlflow.rst Show resolved Hide resolved
@rhettinger rhettinger merged commit 6fcb6cf into python:master Aug 23, 2019
@rhettinger rhettinger deleted the tutorial_control_flow branch August 23, 2019 06:44
@miss-islington
Copy link
Contributor

Thanks @rhettinger for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 23, 2019
(cherry picked from commit 6fcb6cf)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
@bedevere-bot
Copy link

GH-15410 is a backport of this pull request to the 3.8 branch.

rhettinger added a commit that referenced this pull request Aug 23, 2019
(cherry picked from commit 6fcb6cf)

Co-authored-by: Raymond Hettinger <rhettinger@users.noreply.github.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants