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

Add "how to" for the getter Argument Clinic directive. #1232

Merged
merged 16 commits into from
Nov 30, 2023

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Nov 27, 2023

@AlexWaygood
Copy link
Member

@hugovk what was the motivation for dc17090? :) The PR now proposes 0 changes!

@hugovk
Copy link
Member

hugovk commented Nov 27, 2023

@AlexWaygood GitHub UI fail on my part! I've fixed it locally and force pushed! Sorry about that!

@AlexWaygood
Copy link
Member

@AlexWaygood GitHub UI fail on my part! I've fixed it locally and force pushed! Sorry about that!

No worries -- I was briefly worried your account had been hacked! 😅

@hugovk
Copy link
Member

hugovk commented Nov 27, 2023

OK, so I meant to delete a pending review comment:

image

But picked the wrong "..." and chose "Discard changes" here:

image

Lesson learned. Sorry again @corona10!

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.

A few wording suggestions.

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
corona10 and others added 3 commits November 29, 2023 06:47
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
@corona10 corona10 requested a review from AlexWaygood November 30, 2023 03:33
corona10 and others added 2 commits November 30, 2023 20:48
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
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 tiny nits, but overall this looks great now!

@erlend-aasland
Copy link
Contributor

LGTM, given Alex's final remarks are addressed :) Thank you so much, Donghee!

corona10 and others added 2 commits November 30, 2023 22:01
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@corona10
Copy link
Member Author

corona10 commented Nov 30, 2023

Thank you @AlexWaygood, @erlend-aasland, @hugovk and @ezio-melotti :)
Writing in English is quite difficult, but thanks to your feedback, I seem to be improving little by little.

@corona10 corona10 merged commit e8a6577 into python:main Nov 30, 2023
@corona10 corona10 deleted the gh-112205-getter branch November 30, 2023 13:07
@AlexWaygood
Copy link
Member

Thanks so much @corona10, this is great!

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

Successfully merging this pull request may close these issues.

5 participants