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-70795: Rework RLock documentation #103853

Merged
merged 14 commits into from
May 22, 2024

Conversation

samatjain
Copy link
Contributor

@samatjain samatjain commented Apr 25, 2023

Attempted to simultaneously reduce verbosity, while more descriptively describing behavior.

Fix links (RLock acquire/release previously linking to Lock acquire/release, seems like bad copy pasta).

Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Apr 25, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@samatjain
Copy link
Contributor Author

cc @CAM-Gerlach thanks for draft review!

@CAM-Gerlach CAM-Gerlach self-requested a review April 25, 2023 22:34
@CAM-Gerlach CAM-Gerlach added docs Documentation in the Doc dir skip news needs backport to 3.11 only security fixes labels Apr 25, 2023
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

some rewording suggestions.

Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

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.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Thanks! Various suggested changes and fixed.

Standard reminder: You can directly apply all the suggestions you want in one go by going to Files changed -> Clicking Add to batch on each suggestion -> When done, clicking Commit

Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
samatjain and others added 2 commits April 25, 2023 16:39
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

A couple followup comments

Doc/library/threading.rst Outdated Show resolved Hide resolved
Doc/library/threading.rst Outdated Show resolved Hide resolved
samatjain and others added 2 commits April 25, 2023 18:04
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM from my side; thanks @samatjain ! I'll leave it to @gpshead to re-review the content.

@CAM-Gerlach CAM-Gerlach marked this pull request as ready for review April 26, 2023 01:18
@samatjain samatjain requested a review from CAM-Gerlach May 10, 2023 05:58
@serhiy-storchaka serhiy-storchaka added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes and removed needs backport to 3.11 only security fixes labels May 9, 2024
@CAM-Gerlach CAM-Gerlach dismissed gpshead’s stale review May 22, 2024 19:47

All comments appear to be addressed and no further were made after several months

@CAM-Gerlach CAM-Gerlach merged commit 2fbea81 into python:main May 22, 2024
25 checks passed
@miss-islington-app
Copy link

Thanks @samatjain for the PR, and @CAM-Gerlach for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2024
Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).

Add a seealso for with-locks.

Switch section to use bullet points.

---------

(cherry picked from commit 2fbea81)

Co-authored-by: uıɐɾ ʞ ʇɐɯɐs <_@skj.io>
Co-authored-by: Alex Waygood <Alex.Waygood@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 May 22, 2024
Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).

Add a seealso for with-locks.

Switch section to use bullet points.

---------

(cherry picked from commit 2fbea81)

Co-authored-by: uıɐɾ ʞ ʇɐɯɐs <_@skj.io>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119436 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 22, 2024
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119437 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label May 22, 2024
CAM-Gerlach added a commit that referenced this pull request May 22, 2024
gh-70795: Rework RLock documentation (GH-103853)

Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).

Add a seealso for with-locks.

Switch section to use bullet points.

---------

(cherry picked from commit 2fbea81)

Co-authored-by: uıɐɾ ʞ ʇɐɯɐs <_@skj.io>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
gpshead pushed a commit that referenced this pull request May 22, 2024
gh-70795: Rework RLock documentation (GH-103853)

Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).

Add a seealso for with-locks.

Switch section to use bullet points.

---------

(cherry picked from commit 2fbea81)

Co-authored-by: uıɐɾ ʞ ʇɐɯɐs <_@skj.io>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
Attempted to simultaneously reduce verbosity, while more descriptively
describing behavior.

Fix links (RLock acquire/release previously linking to Lock
acquire/release, seems like bad copy pasta).

Add a seealso for with-locks.

Switch section to use bullet points.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
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.

6 participants