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

gh-99991: improve docs on str.encode and bytes.decode #100198

Merged
merged 6 commits into from Dec 21, 2022

Conversation

bizzyvinci
Copy link
Contributor

@bizzyvinci bizzyvinci commented Dec 12, 2022

@netlify
Copy link

netlify bot commented Dec 12, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit 86c44a4
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/63977a667613200008c22058
😎 Deploy Preview https://deploy-preview-100198--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Dec 12, 2022
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Dec 12, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@AlexWaygood AlexWaygood changed the title gh-99991: improve str.encode and byte.decode gh-99991: improve docs on str.encode and byte.decode Dec 13, 2022
@kumaraditya303 kumaraditya303 changed the title gh-99991: improve docs on str.encode and byte.decode gh-99991: improve docs on str.encode and bytes.decode Dec 15, 2022
@kumaraditya303 kumaraditya303 requested a review from CAM-Gerlach Dec 15, 2022
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Thanks. I've added some suggestions to fix grammar errors, better implement the original feedback in the issue, follow our conventions and the Diataxis principles, and clarify and improve the phrasing and markup, as well as a couple comments I couldn't leave as suggestions that you'll need to implement manually (or I can implement as a commit on your branch, if you prefer).

Just FYI, in case you're unfamiliar—you can directly apply some or all of my suggestions by going to the Files changed tab, clicking Add to batch on each suggestion, then clicking Commit with a descriptive message.

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
By default, the *errors* argument is not checked for best performances, but
only used at the first encoding error. Enable the :ref:`Python Development
Copy link
Member

@CAM-Gerlach CAM-Gerlach Dec 16, 2022

Choose a reason for hiding this comment

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

I was not able to actually make this as suggestion, but this implements the revision from the issue, while still including the key details from the previous version.

   For performance reasons, the value of *errors* is not checked for validity
   unless an encoding error actually occurs,
   :ref:`Python Development Mode <devmode>` is enabled
   or a :ref:`debug build <debug-build>` is used.

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
By default, the *errors* argument is not checked for best performances, but
only used at the first decoding error. Enable the :ref:`Python Development
Copy link
Member

@CAM-Gerlach CAM-Gerlach Dec 16, 2022

Choose a reason for hiding this comment

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

I was not able to actually make this as suggestion, but this implements the revision from the issue, while still including the key details from the previous version.

   For performance reasons, the value of *errors* is not checked for validity
   unless a decoding error actually occurs,
   :ref:`Python Development Mode <devmode>` is enabled
   or a :ref:`debug build <debug-build>` is used.

…s.decode more clear

Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bizzyvinci
Copy link
Contributor Author

bizzyvinci commented Dec 17, 2022

Thanks for the suggestions and comments @CAM-Gerlach. And yes, I'll prefer you implement a commit on my branch.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Dec 17, 2022

Done, thanks. I also made a few small refinements to the text of the versionchanged and notes of the same functions in line with the other changes, as well as fixed a missing line break that GitHub stripped.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

LGTM now, thanks @bizzyvinci and @jayaddison !

@JelleZijlstra JelleZijlstra merged commit a2bb3b7 into python:main Dec 21, 2022
15 checks passed
@miss-islington
Copy link
Contributor

miss-islington commented Dec 21, 2022

Thanks @bizzyvinci for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒🤖

@bedevere-bot
Copy link

bedevere-bot commented Dec 21, 2022

GH-100382 is a backport of this pull request to the 3.11 branch.

@bedevere-bot
Copy link

bedevere-bot commented Dec 21, 2022

GH-100383 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 21, 2022
…-100198)

(cherry picked from commit a2bb3b7)

Co-authored-by: Bisola Olasehinde <horlasehinde@gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 21, 2022
…-100198)

(cherry picked from commit a2bb3b7)

Co-authored-by: Bisola Olasehinde <horlasehinde@gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull request Dec 21, 2022
(cherry picked from commit a2bb3b7)

Co-authored-by: Bisola Olasehinde <horlasehinde@gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
miss-islington added a commit that referenced this pull request Dec 21, 2022
(cherry picked from commit a2bb3b7)

Co-authored-by: Bisola Olasehinde <horlasehinde@gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bizzyvinci bizzyvinci deleted the str-docs branch Dec 22, 2022
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.

None yet

5 participants