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

bpo-18697: Update PyUnicode parameter names #12680

Open
wants to merge 7 commits into
base: master
from

Conversation

@CraftSpider
Copy link

commented Apr 4, 2019

Per the linked issue, PyUnicode functions/methods had many different parameter names, this PR aims to unify them. There is still at least one unanswered question on the bug-tracker, but it seemed a good idea to get a PR of the existing fixes started to solicit active comments.

https://bugs.python.org/issue18697

@serhiy-storchaka
Copy link
Member

left a comment

Do not change C sources. Update only the documentation. Don't forget to update the text following the signature.

Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
@CraftSpider

This comment has been minimized.

Copy link
Author

commented Apr 6, 2019

Alright, I'll revert the code changes. Phrasing and discussion on the issue made me think it also wanted the code names unified with the doc names

@csabella csabella requested a review from serhiy-storchaka May 29, 2019

@mangrisano
Copy link
Contributor

left a comment

LGTM. Thank you.

@matrixise
Copy link
Member

left a comment

Thank you for your contribution, but could you check your PR, because you change some parameters, but the documentation is not updated with these new names.

Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
Doc/c-api/unicode.rst Outdated Show resolved Hide resolved
@bedevere-bot

This comment has been minimized.

Copy link

commented Sep 11, 2019

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

CraftSpider added 2 commits Sep 13, 2019
Update references to parameter names
Function descriptions should now all match the changed parameters of their functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.