GeezerVison

View reviewed changes

CAM-Gerlach left a comment

Copy link

Copy Markdown

Member

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.

Looks good overall; just a few comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland marked this pull request as draft

July 7, 2022 20:11

erlend-aasland added the DO-NOT-MERGE label

erlend-aasland commented Jul 7, 2022

Copy link

Copy Markdown

Contributor Author

Thanks for the review, CAM. My first impression when reading the review comments is that this PR is premature; we want to improve the wording in headings, and possibly move around some sections in separate PRs first.

ezio-melotti reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ezio-melotti reviewed

View reviewed changes

ezio-melotti left a comment

Copy link

Copy Markdown

Member

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.

A step in the right direction!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with heart emoji

All reactions

❤️ 1 reaction

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland mentioned this pull request

Restructure the sqlite3 docs table-of-contents #94635

Closed

erlend-aasland mentioned this pull request

Jul 26, 2022

gh-94635: Add sqlite3 'Introduction' and 'Tutorial' doc headings #95269

Merged

erlend-aasland added 3 commits

July 27, 2022 14:39


          Address reviews

3ed9049


          Syntax fix

175f823


          How-to guides a la diataxis

dbc4079

erlend-aasland removed the DO-NOT-MERGE label

Jul 27, 2022

erlend-aasland marked this pull request as ready for review

July 27, 2022 14:06

erlend-aasland requested review from CAM-Gerlach and ezio-melotti

July 27, 2022 14:06

erlend-aasland commented Jul 27, 2022

Copy link

Copy Markdown

Contributor Author

All right, I think this is ready for another round of reviews. I've already added the Introduction and Tutorial headings in a separate PR. For this PR, I restrict it to:

Adding top-level Reference, How-to guides, and Concepts headings, and simply rearrange the existing subsections below these
Normalise heading capitalisation.

Saved for later:

Possibly reword headings (the other draft PR)
Possibly split adapter/converter recipes section

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland changed the title ~~gh-94635: Reorder sqlite3 docs a la Diátaxis~~ gh-94635: Add Reference, How-to, and Concepts headings to sqlite3 docs

Jul 27, 2022

CAM-Gerlach reviewed

Jul 28, 2022

View reviewed changes

CAM-Gerlach left a comment

Copy link

Copy Markdown

Member

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.

A few minor textual suggestions, otherwise LGTM, thanks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Partially adress CAM's review

c529b7f

CAM-Gerlach reviewed

Jul 29, 2022

View reviewed changes

CAM-Gerlach left a comment

Copy link

Copy Markdown

Member

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.

Just a couple minor followups, otherwise LGTM, thanks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with hooray emoji

All reactions

🎉 1 reaction

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Update Doc/library/sqlite3.rst

Co-authored-by: CAM Gerlach <[email protected]>


          Update Doc/library/sqlite3.rst

d4cdbee

Co-authored-by: CAM Gerlach <[email protected]>

erlend-aasland commented Jul 29, 2022

Copy link

Copy Markdown

Contributor Author

Thanks for the reviews! I'll let this sit around for some more days before merging, to give @ezio-melotti a chance to review the final changes.

CAM-Gerlach reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

CAM-Gerlach approved these changes

Jul 29, 2022

View reviewed changes

CAM-Gerlach left a comment

Copy link

Copy Markdown

Member

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 on my end, thanks @erlend-aasland

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with hooray emoji

All reactions

🎉 1 reaction

ezio-melotti approved these changes

View reviewed changes

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/sqlite3.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting merge and removed awaiting core review labels

Erlend Egeberg Aasland and others added 3 commits

July 30, 2022 23:32


          Don't link to the module inside the module docs

d53d11d


          Merge remote-tracking branch 'upstream/main' into sqlite-diataxis

12854e1


          Fix merge

56e0139

erlend-aasland added needs backport to 3.10 needs backport to 3.11 only security fixes labels

erlend-aasland commented Jul 30, 2022

Copy link

Copy Markdown

Contributor Author

Thanks for the reviews, CAM and Ezio!

ezio-melotti reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland merged commit 6c439b9 into python:main

miss-islington commented Jul 30, 2022

Copy link

Copy Markdown

Contributor

Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the awaiting merge label

erlend-aasland deleted the sqlite-diataxis branch

July 30, 2022 23:24

miss-islington commented Jul 30, 2022

Copy link

Copy Markdown

Contributor

Sorry, @erlend-aasland, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 6c439b978bf55b29f21c595e9375202d63c8208d 3.11

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington assigned erlend-aasland

miss-islington commented Jul 30, 2022

Copy link

Copy Markdown

Contributor

Sorry @erlend-aasland, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 6c439b978bf55b29f21c595e9375202d63c8208d 3.10

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request


          [3.11] pythongh-94635: Add Reference, How-to, and Concepts headings t…

2d05648

…o sqlite3 docs (pythonGH-94636)

Co-authored-by: CAM Gerlach <[email protected]>.
(cherry picked from commit 6c439b9)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

bedevere-bot removed the needs backport to 3.11 only security fixes label

bedevere-bot commented Jul 30, 2022

Copy link

Copy Markdown

GH-95482 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot commented Jul 30, 2022

Copy link

Copy Markdown

GH-95483 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.10 label

erlend-aasland added a commit to erlend-aasland/cpython that referenced this pull request


          [3.10] pythongh-94635: Add Reference, How-to, and Concepts headings t…

081571d

…o sqlite3 docs (pythonGH-94636)

Co-authored-by: CAM Gerlach <[email protected]>.
(cherry picked from commit 6c439b9)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

erlend-aasland added a commit that referenced this pull request


          [3.11] gh-94635: Add Reference, How-to, and Explanation headings to s…

c7ac8b6

…qlite3 docs (GH-94636) (#95482)

Co-authored-by: CAM Gerlach <[email protected]>.
(cherry picked from commit 6c439b9)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

* Fix refs

erlend-aasland added a commit that referenced this pull request


          [3.10] gh-94635: Add Reference, How-to, and Explanation headings to s…

6765a78

…qlite3 docs (GH-94636) (#95483)

Co-authored-by: CAM Gerlach <[email protected]>.
(cherry picked from commit 6c439b9)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

* Fix refs

ezio-melotti mentioned this pull request

Style guide suggestion: Avoid duplicate links python/docs-community#52

Closed

CAM-Gerlach reviewed