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-95273: Align sqlite3 const refs with the devguide#95525
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-95273: Align sqlite3 const refs with the devguide #95525
Uh oh!
There was an error while loading. Please reload this page.
Conversation
erlend-aasland commented Aug 1, 2022
OTOH, adding |
ezio-melotti 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.
As discussed in python/devguide#916 (comment), using ``...`` is preferable to :const:`!...`.
erlend-aasland commented Aug 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.
Thanks, Ezio! I forgot to change the parameter list types1; I'll update those before merging. OTOH, I'm not sure about changing those; maybe best to leave those to the Footnotes
|
CAM-Gerlach commented Aug 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.
That's what I would do (and have done in my PRs doing this elsewhere) as its simpler, consistent with standard Sphinxdoc usage and the structure makes its semantics unambiguous without the need for additional formatting, particularly since the context is as a type, not a code literal. If we need to customize it, we could monkeypatch the directive or submit an upstream PR instead of manually doing every one. |
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.
I regexed the doc, built it and and spot-checked the preview, and this LGTM, thanks @erlend-aasland
erlend-aasland commented Aug 3, 2022
Yes, you spelled out my thoughts about this, CAM :) Also, +1 to this:
Thanks for the review, both of you. Highly appreciated! I'll land this. |
miss-islington commented Aug 3, 2022
Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
miss-islington commented Aug 3, 2022
Sorry, @erlend-aasland, I could not cleanly backport this to |
bedevere-bot commented Aug 3, 2022
GH-95616 is a backport of this pull request to the 3.11 branch. |
…endations (pythonGH-95525) (cherry picked from commit 4d02572) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
… recommendations (pythonGH-95525). (cherry picked from commit 4d02572) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>
bedevere-bot commented Aug 3, 2022
GH-95618 is a backport of this pull request to the 3.10 branch. |
CAM-Gerlach commented Aug 3, 2022
Probably less concisely than you would have, though 😆 |
Uh oh!
There was an error while loading. Please reload this page.