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.

Sign up for GitHub

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

Jump to bottom

PEP 688: Making the buffer protocol accessible in Python #2549

Merged
merged 18 commits into from
Apr 25, 2022
Merged

PEP 688: Making the buffer protocol accessible in Python #2549

merged 18 commits into from
Apr 25, 2022

Conversation

JelleZijlstra
Copy link
Member

No description provided.

@JelleZijlstra JelleZijlstra requested a review from a team as a code owner April 23, 2022 16:48
AA-Turner
AA-Turner reviewed Apr 23, 2022
View reviewed changes
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

There are two uses of "I" which I've suggested turning into plural or neutral language as the rest of the document seems to use "we".

A couple of small suggestions elsewise.

A

pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
@CAM-Gerlach CAM-Gerlach added the new-pep A new draft PEP submitted for initial review label Apr 23, 2022
@CAM-Gerlach
Copy link
Member

Hey @JelleZijlstra , would you like me to proofread and suggest copyedits on this?

@JelleZijlstra
Copy link
Member Author

@CAM-Gerlach yes please! Can't guarantee I'll agree with all your suggestions though :)

@JelleZijlstra JelleZijlstra mentioned this pull request Apr 24, 2022
Closed
CAM-Gerlach
CAM-Gerlach reviewed Apr 24, 2022
View reviewed changes
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Here you go! In a lot of places, I provided multiple different possible options for you to consider that resolved an issue, if you didn't like the first one (or I myself had no strong preference).

pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood reviewed Apr 24, 2022
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

I'm excited to see this happen! A few thoughts from me below :)

pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood reviewed Apr 24, 2022
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

A few more small points :)

pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
pep-0688.rst Outdated Show resolved Hide resolved
AlexWaygood
AlexWaygood approved these changes Apr 25, 2022
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Looks great to me :)

@JelleZijlstra JelleZijlstra merged commit 0cb0066 into python:main Apr 25, 2022
@JelleZijlstra JelleZijlstra deleted the bufferpep branch April 25, 2022 03:39
@JelleZijlstra
Copy link
Member Author

Thanks everyone for the feedback! I just merged the PR so it shows up on the official site. If you have further textual suggestions, feel free to put a note here, open a PR, give me a handwritten note at PyCon, or let me know in whatever other way you can think of.

@gvanrossum
Copy link
Member

Carrier pigeon.

@CAM-Gerlach
Copy link
Member

Interpretive dance.

@erlend-aasland erlend-aasland mentioned this pull request May 13, 2022
Closed
@erlend-aasland erlend-aasland mentioned this pull request Jun 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AA-Turner AA-Turner AA-Turner left review comments

@CAM-Gerlach CAM-Gerlach CAM-Gerlach left review comments

@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees
No one assigned
Labels
new-pep A new draft PEP submitted for initial review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@JelleZijlstra @CAM-Gerlach @gvanrossum @AA-Turner @Fidget-Spinner @AlexWaygood @hauntsaninja