Skip to content

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

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
Merged
corona10 merged 16 commits into from
Nov 30, 2023
Merged

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

corona10 merged 16 commits into from
Nov 30, 2023

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Nov 27, 2023•
edited by github-actions bot
Loading

@corona10corona10 mentioned this pull request Nov 27, 2023
Merged
@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 27, 2023

@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 commented Nov 27, 2023

@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!

hugovk
hugovk reviewed Nov 27, 2023
View reviewed changes
Copy link
Member

@hugovkhugovk 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.

@corona10@hugovk
Apply suggestions from code review
97e644e
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
erlend-aasland
erlend-aasland reviewed Nov 28, 2023
View reviewed changes
erlend-aasland
erlend-aasland reviewed Nov 28, 2023
View reviewed changes
corona10and others added 3 commits November 29, 2023 06:47
@corona10@erlend-aasland
Update development-tools/clinic.rst
8cb31a9
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@corona10
Address code review
1af671f
@corona10
fix
d631e8e
ezio-melotti
ezio-melotti reviewed Nov 29, 2023
View reviewed changes
@corona10@ezio-melotti
Apply suggestions from code review
7a8bfae
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
AlexWaygood
AlexWaygood reviewed Nov 29, 2023
View reviewed changes
AlexWaygood
AlexWaygood reviewed Nov 30, 2023
View reviewed changes
erlend-aasland
erlend-aasland reviewed Nov 30, 2023
View reviewed changes
corona10and others added 2 commits November 30, 2023 20:48
@corona10@erlend-aasland@AlexWaygood
Apply suggestions from code review
5f48ac0
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@corona10
fix
2b2d38b
AlexWaygood
AlexWaygood reviewed Nov 30, 2023
View reviewed changes
Copy link
Member

@AlexWaygoodAlexWaygood 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

erlend-aasland commented Nov 30, 2023

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

corona10and others added 2 commits November 30, 2023 22:01
@corona10@AlexWaygood
Apply suggestions from code review
a630aec
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@corona10@AlexWaygood
Update development-tools/clinic.rst
8d5ffd1
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@corona10
Copy link
MemberAuthor

corona10 commented Nov 30, 2023•
edited
Loading

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.

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

AlexWaygood commented Nov 30, 2023

Thanks so much @corona10, this is great!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovkhugovkhugovk left review comments

@ezio-melottiezio-melottiezio-melotti left review comments

@AlexWaygoodAlexWaygoodAlexWaygood left review comments

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@corona10@AlexWaygood@hugovk@erlend-aasland@ezio-melotti