Skip to content

Conversation

@willingc
Copy link
Contributor

@willingc willingc commented Feb 28, 2021

@willingc
Copy link
Contributor Author

@ramalho If you get a chance, please take a look. Thanks!

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thank you so much Carol! I found a few markup mistakes and have a few more editorial suggestions.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I hit Submit too soon, here are the last three review items. Goodnight!

Copy link
Member

@Fidget-Spinner Fidget-Spinner 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 read everything in detail, but here's a bunch of suggestions to get the CI checks to pass :).
(mostly just whitespace fixes)

Copy link

@KunKax KunKax left a comment

Choose a reason for hiding this comment

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

.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

+1 on having Scala *and *Elixir.

@ramalho
Copy link
Contributor

ramalho commented Feb 28, 2021

Great quick intro, @willingc thanks! I think the special variable _ deserves more attention. It appears only alone in the last case clause, and then appears again in a discussion of *. Adding an example where _ appears inside a more elaborate pattern without * would clarify the essential meaning of _. Something like ('error', code, _).

I think it would help readers if that example could appear after an example with case _, but before any mention of *.

Also, as @gvanrossum suggested, it's important to have an example without case _ to make it clear that no match means no op.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@willingc
Copy link
Contributor Author

Thanks @gvanrossum, @ramalho, and others who helped review this PR. Your feedback improved the content which should help users with understanding.

@gvanrossum
Copy link
Member

You're welcome! @willingc Go ahead and land it.

@willingc willingc merged commit 41934b3 into python:master Feb 28, 2021
@bedevere-bot
Copy link

@willingc: Please replace # with GH- in the commit message next time. Thanks!

@willingc willingc deleted the patterns branch February 28, 2021 23:43
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.

8 participants