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

Swap 'if' branches so content matches to condition in importlib example #14947

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

uranusjr
Copy link
Contributor

@uranusjr uranusjr commented Jul 25, 2019

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon

With the previous version, when I see `spec` referenced in the else
block, I had to trace all the way back to the previous elif condition to
find the assignment, and read the elif block again to make sure the
variable is not changed inside.

This implementation reads much more cleaner to me since `spec` is used
immediately after it's introduced, and I no longer need to look it back
up again after the elif block.
@tirkarthi
Copy link
Member

cc: @brettcannon

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Me likey this more.

Copy link
Contributor

@mental32 mental32 left a comment

Choose a reason for hiding this comment

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

👍 This reads cleaner

@brettcannon brettcannon changed the title Swap if branches so content matches to condition Swap 'if' branches so content matches to condition in importlib example Jul 25, 2019
@miss-islington
Copy link
Contributor

@uranusjr: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 544fa15 into python:master Jul 25, 2019
@miss-islington
Copy link
Contributor

Thanks @uranusjr 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 Jul 25, 2019
…le (pythonGH-14947)

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon
(cherry picked from commit 544fa15)

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@bedevere-bot
Copy link

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

miss-islington added a commit that referenced this pull request Jul 25, 2019
…le (GH-14947)

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon
(cherry picked from commit 544fa15)

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@uranusjr uranusjr deleted the doc-importlib-swap-walrus branch July 25, 2019 21:22
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
…le (pythonGH-14947)

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
…le (pythonGH-14947)

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
…le (pythonGH-14947)

Prior to this change the guard on an 'elif' used an assignment expression whose value was used in a later 'else' block, causing some confusion for people.

(Discussion on Twitter: https://twitter.com/brettsky/status/1153861041068994566.)

Automerge-Triggered-By: @brettcannon
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 issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants