Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-94017: Improve clarity of sqlite3 transaction handling docs#94320
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-94017: Improve clarity of sqlite3 transaction handling docs #94320
Uh oh!
There was an error while loading. Please reload this page.
Conversation
erlend-aasland commented Jun 27, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Jun 27, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
cc. @CAM-Gerlach, @AlexWaygood. Would you two mind reviewing this? I'll go over it later today, to be sure the docs align with the actual implementation. Other that, I worry that the new tone of the docs may come across as ... perhaps a trifle strict, or negative sounding (in lack of more appropriate words). I also worry about the added verbosity. I do hope this makes things clearer, though. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
CAM-Gerlach left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @erlend-aasland , this is a definite improvement. I do have some clarity, textual and reST fixes and suggestions.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
AlexWaygood left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to most of Cam's thoughts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Jul 1, 2022
Thanks, both of you; highly appreciated. I'm finally getting around to address the reviews. |
erlend-aasland commented Jul 1, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Reviews addressed in a530b3e. |
CAM-Gerlach commented Jul 2, 2022
That's fine with me, since it isn't used on multiple classes in the module and links directly to them in either case. I wasn't very consistent about using that myself. |
CAM-Gerlach left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @erlend-aasland
I had one comment, but that might be out of scope here and better addressed in a future PR.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Jul 3, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I'm sorry, but I did a last rewrite of things, trying to address #94320 (comment). Hopefully to the better. I'd be happy if you could review commit 1473e94. If it was for the worse, I'll revert it. For the "Controlling Transactions" section, I tried to improve the visual separation of the two sqlite3 transaction modes. |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Alex Waygood <[email protected]>
CAM-Gerlach commented Jul 5, 2022
No need to apologize for doing more work and making the docs better! I'll review it now—sorry myself for the delay; it was a holiday weekend and I was with family. |
CAM-Gerlach left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review feedback on the final revisisions
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: CAM Gerlach <[email protected]>
erlend-aasland commented Jul 6, 2022
Thanks again for the reviews; I'll land this later tonight. |
miss-islington commented Jul 6, 2022
Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
…ythonGH-94320) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: CAM Gerlach <[email protected]> (cherry picked from commit 760b8cf) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
bedevere-bot commented Jul 6, 2022
GH-94617 is a backport of this pull request to the 3.11 branch. |
…ythonGH-94320) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: CAM Gerlach <[email protected]> (cherry picked from commit 760b8cf) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
bedevere-bot commented Jul 6, 2022
GH-94618 is a backport of this pull request to the 3.10 branch. |
) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: CAM Gerlach <[email protected]> (cherry picked from commit 760b8cf) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: CAM Gerlach <[email protected]> (cherry picked from commit 760b8cf) Co-authored-by: Erlend Egeberg Aasland <[email protected]>
Fixesgh-94017