Skip to content

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

Merged
corona10 merged 16 commits intopython:mainfrom
corona10:gh-112205-getter
Nov 30, 2023
Merged

Add "how to" for the getter Argument Clinic directive.#1232
corona10 merged 16 commits intopython:mainfrom
corona10:gh-112205-getter

Conversation

@corona10
Copy link
Member

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

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

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

@corona10 @hugovk
Apply suggestions from code review
97e644e 
Co-authored-by: Hugo van Kemenade <[email protected]>
erlend-aasland
erlend-aasland reviewed Nov 28, 2023
View reviewed changes
erlend-aasland
erlend-aasland reviewed Nov 28, 2023
View reviewed changes
ezio-melotti
ezio-melotti reviewed Nov 29, 2023
View reviewed changes
AlexWaygood
AlexWaygood reviewed Nov 29, 2023
View reviewed changes
@corona10 corona10 requested a review from AlexWaygood November 30, 2023 03:33
AlexWaygood
AlexWaygood reviewed Nov 30, 2023
View reviewed changes
erlend-aasland
erlend-aasland reviewed Nov 30, 2023
View reviewed changes
corona10 and 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 <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@corona10
fix
2b2d38b
AlexWaygood
AlexWaygood reviewed Nov 30, 2023
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 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!

@corona10
Copy link
Member Author

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.

@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

AlexWaygood commented Nov 30, 2023

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

Reviewers

@hugovk hugovk hugovk left review comments

@ezio-melotti ezio-melotti ezio-melotti left review comments

@AlexWaygood AlexWaygood AlexWaygood left review comments

@erlend-aasland erlend-aasland erlend-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