Skip to content

bpo-38351: Modernize email example from %-formatting to f-string#17162

Merged
taleinat merged 3 commits intopython:masterfrom
dorosch:modernize_email_examples
Nov 15, 2019
Merged

bpo-38351: Modernize email example from %-formatting to f-string#17162
taleinat merged 3 commits intopython:masterfrom
dorosch:modernize_email_examples

Conversation

@dorosch
Copy link
Copy Markdown
Contributor

@dorosch dorosch commented Nov 15, 2019 •
edited by bedevere-bot
Loading

@dorosch dorosch requested a review from a team as a code owner November 15, 2019 06:21
@dorosch dorosch force-pushed the modernize_email_examples branch from 301b3b0 to 9583f64 Compare November 15, 2019 06:23
@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

Thanks for the PR @dorosch!

@brandtbucher brandtbucher added the docs Documentation in the Doc dir label Nov 15, 2019
brandtbucher
brandtbucher requested changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

I agree these should be updated, but f-strings are actually templated string literals (str.format is a third, different way of formatting strings).

You're almost there though... see my suggestions below:

@dorosch
Copy link
Copy Markdown
Contributor Author

dorosch commented Nov 15, 2019 •
edited
Loading

I agree these should be updated, but f-strings are actually templated string literals (str.format is a third, different way of formatting strings).

You're almost there though... see my suggestions below:

Thanks for your feedback. I used str.format as in other examples of documentation used str.format

brandtbucher
brandtbucher requested changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Great, just a couple more comments!

@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

The CI failure is unrelated. Pushing to this branch should trigger a new build.

aeros
aeros suggested changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @dorosch and welcome! Other than a few minor details, the PR looks most of the way there after @brandtbucher's suggestions.

@aeros
Copy link
Copy Markdown
Contributor

aeros commented Nov 15, 2019 •
edited
Loading

It looks like the travis build failure was not related to the PR, committing the latest suggestions will restart the CI (or closing and reopening the PR).

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

Looks like we crossed posts @aeros. 🙂

@aeros
Copy link
Copy Markdown
Contributor

aeros commented Nov 15, 2019

Looks like we crossed posts @aeros. slightly_smiling_face

Haha, oops. 🙂

@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

Good to see our comments are almost entirely identical. Haha.

@aeros
Copy link
Copy Markdown
Contributor

aeros commented Nov 15, 2019

These changes can be backported to the 3.8 and 3.7 docs as well.

@dorosch
Copy link
Copy Markdown
Contributor Author

dorosch commented Nov 15, 2019

These changes can be backported to the 3.8 and 3.7 docs as well.

Could you tell me how I can do this?

@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

That's a note for the core reviewer. Nothing required on your part @dorosch!

brandtbucher
brandtbucher approved these changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Awesome.

aeros
aeros approved these changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for applying the suggested changes @dorosch.

@aeros
Copy link
Copy Markdown
Contributor

aeros commented Nov 15, 2019 •
edited
Loading

@dorosch

Could you tell me how I can do this?

@brandtbucher

That's a note for the core reviewer. Nothing required on your part @dorosch!

Correct, but more specifically: when the needs backport to <x> label is applied, the bot @miss-islington will attempt to automatically open a backport PR to the branch for version <x>, after the original PR is merged. Sometimes when there are merge conflicts, it is required to use the cherry-picker tool (https://pypi.org/project/cherry-picker/) to resolve them and manually open the backport PR(s), but most of the time it can be done automatically (especially for minor changes).

@dorosch
Copy link
Copy Markdown
Contributor Author

dorosch commented Nov 15, 2019

@dorosch

Could you tell me how I can do this?

@brandtbucher

That's a note for the core reviewer. Nothing required on your part @dorosch!

Correct, but more specifically: when the needs backport to <x> label is applied, the bot @miss-islington will attempt to automatically open a backport PR to the branch for version <x>, after the original PR is merged. Sometimes when there are merge conflicts, it is required to use the cherry-picker tool (https://pypi.org/project/cherry-picker/) to resolve them and manually open the backport PR(s), but most of the time it can be done automatically (especially for minor changes).

Thanks for your explanation!

taleinat
taleinat approved these changes Nov 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@taleinat taleinat left a comment

Choose a reason for hiding this comment

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

LGTM

@taleinat taleinat merged commit e8acc86 into python:master Nov 15, 2019
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Nov 15, 2019

Thanks @dorosch for the PR, and @taleinat for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 15, 2019
@dorosch @miss-islington
bpo-38351: Modernize email examples from %-formatting to f-strings (p…
3ba0fdd 
…ythonGH-17162)

(cherry picked from commit e8acc86)

Co-authored-by: Andrey Doroschenko <dorosch.github.io@yandex.ru>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Nov 15, 2019

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

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Nov 15, 2019

GH-17168 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 15, 2019
@dorosch @miss-islington
bpo-38351: Modernize email examples from %-formatting to f-strings (p…
037f8b4 
…ythonGH-17162)

(cherry picked from commit e8acc86)

Co-authored-by: Andrey Doroschenko <dorosch.github.io@yandex.ru>
miss-islington added a commit that referenced this pull request Nov 15, 2019
@miss-islington @dorosch
bpo-38351: Modernize email examples from %-formatting to f-strings (G…
dae27cc 
…H-17162)

(cherry picked from commit e8acc86)

Co-authored-by: Andrey Doroschenko <dorosch.github.io@yandex.ru>
miss-islington added a commit that referenced this pull request Nov 15, 2019
@miss-islington @dorosch
bpo-38351: Modernize email examples from %-formatting to f-strings (G…
fe67a53 
…H-17162)

(cherry picked from commit e8acc86)

Co-authored-by: Andrey Doroschenko <dorosch.github.io@yandex.ru>
@brandtbucher
Copy link
Copy Markdown
Member

brandtbucher commented Nov 15, 2019

Congrats on your first CPython contribution @dorosch! 🍾

Looking forward to seeing more from you in the future.

@dorosch
Copy link
Copy Markdown
Contributor Author

dorosch commented Nov 15, 2019

Congrats on your first CPython contribution @dorosch! champagne

Looking forward to seeing more from you in the future.

@brandtbucher Thank you very much for your help

jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@dorosch
bpo-38351: Modernize email examples from %-formatting to f-strings (p…
8073165 
…ythonGH-17162)
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@dorosch @shihai1991
bpo-38351: Modernize email examples from %-formatting to f-strings (p…
f847ee9 
…ythonGH-17162)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericvsmith ericvsmith ericvsmith approved these changes

@taleinat taleinat taleinat approved these changes

@brandtbucher brandtbucher brandtbucher approved these changes

+1 more reviewer

@aeros aeros aeros approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@dorosch @brandtbucher @aeros @miss-islington @bedevere-bot @ericvsmith @taleinat @the-knights-who-say-ni