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

PEP 676: Formatting and prose updates #2207

Merged
merged 3 commits into from
Jan 9, 2022

Conversation

AA-Turner
Copy link
Member

xref #2200

This is the consolidated version I suggested I would write.

I couldn't work out how to keep @CAM-Gerlach as the 'author' of the formatting commit whilst re-writing history, so I added him as co-author on the commits with the Co-authored-by: syntax.

This keeps all the formatting and spelling fixes (two line breaks before a section, all references to use `link`_), and merges the prose updates.

I would like to have this resolved before posting the PEP to python-dev, so if anyone (esp @CAM-Gerlach / @hugovk ) has any comments, please would you let me know sharpish!

I also have a 2 part implementation updates PR I'm preparing in a few moments, will link that here when I send it.

A

@AA-Turner
Copy link
Member Author

xref #2208 and #2209 for the implementation update PRs.

A

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

👍

@Mariatta
Copy link
Member

Mariatta commented Jan 4, 2022

Looks fine to me, though now there's a merge conflict. Would you be able to address the conflict?

@CAM-Gerlach
Copy link
Member

I've got to get on a flight now, but I'll try to look over it late tonight or early tomorrow; sorry for the long delay—had a complex trip to visit various family members, with very limited time and internet over the past couple weeks. So far, it looks very good though.

Looks like the merge conflict is due to #2211 which fixes a bunch of trivial typos in a smattering of PEPs...which is already fixed in this PR (as well as #2200 ) and thus conflicts with both, and possibly others...which the automated implementation in #2151 would help address, but I digress.

I couldn't work out how to keep @CAM-Gerlach as the 'author' of the formatting commit whilst re-writing history, so I added him as co-author on the commits with the Co-authored-by: syntax.

Unacceptable, I say. Unacceptable! 😤 /s

Thanks for thinking of that, and no worries at all—it was as much your work as mine! As a note for the future, for situations where this is important, you can either use the GIT_* env vars or, as a hack, steal one of my original commits, apply the patch to the working tree and then --amend it it into that commit—it will list you as commiter (as it should), but I'll still be listed as the author. But in this case, that's way too much work for something that doesn't really matter—in fact, I think it makes more sense for you to be the author anyway, since you did manually author the changes.

AA-Turner and others added 3 commits January 5, 2022 08:26
Co-authored-by: C.A.M. Gerlach <cam.gerlach@gerlach.cam>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <cam.gerlach@gerlach.cam>
@AA-Turner
Copy link
Member Author

@Mariatta conflict resolved -- thanks for pointing it out

A

@AA-Turner
Copy link
Member Author

@CAM-Gerlach quick ping :) I don't know if you've had any time to review?

use the GIT_* env vars

I had no idea these existed! Learn a new thing every day...

A

@JelleZijlstra JelleZijlstra merged commit 19684a0 into python:main Jan 9, 2022
@AA-Turner AA-Turner deleted the pep-676-update branch January 9, 2022 16:37
@CAM-Gerlach
Copy link
Member

Sorry, busy with holiday travel and visiting family, but from what I skimmed it looked fine to me, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants