bpo-38413: Improve documentation of sqlite3 module by greatvovan · Pull Request #23159 · python/cpython

greatvovan · 2020-11-05T02:00:11Z

This removes an outdated section in the documentation and slightly improves wording and formatting.

See https://bugs.python.org/issue38413

https://bugs.python.org/issue38413

Remove outdate note about multithreading in sqlite3

Improve wording and formatting in the footnote.

greatvovan · 2020-11-05T02:15:27Z

Sorry, I am new to this repo, how can I assign "skip news" label?

dimaqq · 2020-11-05T04:04:59Z

Sorry, I am new to this repo, how can I assign "skip news" label?

AFAIK, someone has to do that for you.

github-actions · 2020-12-16T00:27:30Z

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

dimaqq · 2020-12-16T01:12:22Z

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

No, it's not!

greatvovan · 2020-12-16T04:01:54Z

I am the author. Is anything else required from me? Any concerns?

greatvovan · 2020-12-18T23:25:16Z

Anybody? @berkerpeksag, it shows you as code owner... Is there anything that needs to be fixed?

greatvovan · 2020-12-30T01:08:01Z

@willingc please help.

willingc · 2021-01-05T00:54:42Z

@greatvovan I've been offline for a few weeks. I'll add this to my queue for review. Thanks for your patience.

bedevere-bot · 2021-01-05T02:53:50Z

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.

Revert to mentioning "configure" script.

Keep original lines

greatvovan · 2021-01-05T17:09:49Z

Thanks @berkerpeksag, I reverted to mentioning configure script and added the link you provided. I insist that the original sentence is bad as it gives no hints to the topic 90% of Python developers will not ever touch.

Please also note that the main issue here is removing a paragraph that is inconsistent with other article sections (see BPO), it is not a "random formatting change".

greatvovan · 2021-01-05T22:32:16Z

I think it worths to mention that Python must be rebuilt (recompiled) as just running the script will do nothing.

berkerpeksag · 2021-01-05T23:36:48Z

Let me try again. You are trying to make two separate changes in a single PR:

Removing an outdated section in the documentation. This is completely fine and I'd be happy to merge such PR.
Making random formatting and wording changes on a completely unrelated sentence explaining something you don't fully understand. This is not okay for the following reasons:
- We prefer doing one change (this can be a new feature, bug fix, documentation changes) per BPO issue. bpo-38413 is about removing an outdated section in the sqlite3 documentation. You cannot include another change. It's a bad practice. If you found something else while working on bpo-38413, the best practice is to open a new BPO issue or submit a PR (for example, if you found a typo, you can just submit a separate PR without creating an issue)
- The change you are trying to insist on including is incorrect and the old sentence hasn't been caused any confusion for the past decade. So I'm not going to change it just because someone who initially proposed replacing configure with pyenv's PYTHON_CONFIGURE_OPTS said so.
- It's okay to not know about a very niche feature. We all learn something new all the time. However, it's not okay to keep pushing for it when someone else said "no your change is incorrect, please revert to the old version."

Revert back some changes unrelated to the BPO.

berkerpeksag

LGTM, thank you!

miss-islington · 2021-01-06T00:18:49Z

Thanks @greatvovan for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington · 2021-01-06T00:18:55Z

Sorry @greatvovan and @berkerpeksag, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker f9949f82e17c88609adb53eff3a7d5cd63a645bd 3.9

bedevere-bot · 2021-01-06T00:19:09Z

GH-24134 is a backport of this pull request to the 3.8 branch.

…ythonGH-23159) (cherry picked from commit f9949f8) Co-authored-by: Vladimir <greatvovan@gmail.com>

berkerpeksag · 2021-01-06T00:20:51Z

@greatvovan do you have time to submit a backport PR for 3.9? I can help you if you don't want to use cherry_picker.

…H-23159) (cherry picked from commit f9949f8) Co-authored-by: Vladimir <greatvovan@gmail.com>

greatvovan · 2021-01-06T00:26:26Z

I admitted that I was incorrect in my initial change. With your help it was fixed, this is why we have review process of pull requests. I am still not getting what is wrong with the second version and why you are still saying I don't understand something two times in one post.
I got your point about creating a separate BPO/PR for other unrelated issues and am actually agree that this is better, but in this case I believe it's easier to just inform your counterpart about best practices for the future and proceed. Due to such kind of bureaucracy you have 1.5K of open pull requests.
You have no data to state that "the old sentence hasn't been caused any confusion for the past decade". Not everyone who finds a confusing phrase runs submitting a change. Questions about enabling sqlite extensions can be found on StackOverflow and in other places, so it is indeed calls confusion.
I reverted the original sentence. You just left the Python documentation with this ugly phrase:

you must pass --enable-loadable-sqlite-extensions to configure

My congratulations.

greatvovan · 2021-01-06T00:28:10Z

do you have time to submit a backport PR for 3.9?

Yes, I do have time, but your help is appreciated (links, recommendations). Isn't it just a PR to another branch with the same change?

berkerpeksag · 2021-01-06T00:46:05Z

Isn't it just a PR to another branch with the same change?

Yes, you basically do:

$ git checkout -b backport-to-3.9 3.9
$ git cherry-pick -x f9949f82e17c88609adb53eff3a7d5cd63a645bd

And add [3.9] prefix to PR title when submitting it. You can copy and paste the title of GH-24134 and replace 3.8 with 3.9.

…ythonGH-23159) (cherry picked from commit f9949f8)

bedevere-bot · 2021-01-06T19:26:25Z

GH-24145 is a backport of this pull request to the 3.9 branch.

greatvovan · 2021-01-06T19:27:56Z

@berkerpeksag #24145

Edit: oh, the bot is faster than me...

…H-23159) (cherry picked from commit f9949f8)

…ythonGH-23159)

greatvovan added 2 commits October 28, 2020 00:55

Fix bpo-38413
cf0436d
Remove outdate note about multithreading in sqlite3

Update sqlite3.rst
cb23459
Improve wording and formatting in the footnote.

greatvovan requested a review from berkerpeksag as a code owner November 5, 2020 02:00

the-knights-who-say-ni added the CLA signed label Nov 5, 2020

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 5, 2020

dimaqq approved these changes Nov 5, 2020
View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Nov 5, 2020

greatvovan marked this pull request as draft November 5, 2020 02:11

greatvovan marked this pull request as ready for review November 5, 2020 02:14

github-actionsBot added the stale Stale PR or inactive for long period of time. label Dec 16, 2020

github-actionsBot removed the stale Stale PR or inactive for long period of time. label Dec 17, 2020

willingc self-requested a review January 5, 2021 00:54

berkerpeksag requested changes Jan 5, 2021
View reviewed changes

Comment threadDoc/library/sqlite3.rst Outdated

bedevere-bot removed the awaiting core review label Jan 5, 2021

bedevere-bot added the awaiting changes label Jan 5, 2021

greatvovan added 2 commits January 5, 2021 09:00

Update sqlite3.rst
ced42cd
Revert to mentioning "configure" script.

Update sqlite3.rst
9a04f52
Keep original lines

greatvovan requested a review from berkerpeksag January 5, 2021 17:11

Update sqlite3.rst
c6e5f91
Revert back some changes unrelated to the BPO.

berkerpeksag approved these changes Jan 6, 2021
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Jan 6, 2021

berkerpeksag added needs backport to 3.8 skip news labels Jan 6, 2021

berkerpeksag merged commit f9949f8 into python:masterJan 6, 2021

bedevere-bot removed the awaiting merge label Jan 6, 2021

miss-islington assigned berkerpeksag Jan 6, 2021

bedevere-bot removed the needs backport to 3.8 label Jan 6, 2021

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 6, 2021
bpo-38413: Remove outdated section about multithreading in sqlite3 (p…
a7a06a1
…ythonGH-23159) (cherry picked from commit f9949f8) Co-authored-by: Vladimir <greatvovan@gmail.com>

berkerpeksag pushed a commit that referenced this pull request Jan 6, 2021
bpo-38413: Remove outdated section about multithreading in sqlite3 (G…
03a079f
…H-23159) (cherry picked from commit f9949f8) Co-authored-by: Vladimir <greatvovan@gmail.com>

greatvovan added a commit to greatvovan/cpython that referenced this pull request Jan 6, 2021
bpo-38413: Remove outdated section about multithreading in sqlite3 (p…
e842387
…ythonGH-23159) (cherry picked from commit f9949f8)

bedevere-bot removed the needs backport to 3.9 label Jan 6, 2021

berkerpeksag pushed a commit that referenced this pull request Jan 6, 2021
bpo-38413: Remove outdated section about multithreading in sqlite3 (G…
49c150f
…H-23159) (cherry picked from commit f9949f8)

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
bpo-38413: Remove outdated section about multithreading in sqlite3 (p…
082a0a6
…ythonGH-23159)

erlend-aasland mentioned this pull request Jul 27, 2022
Improve the sqlite3 reference #95273
Closed
5 tasks

Uh oh!

Uh oh!

Conversation

greatvovan commented Nov 5, 2020• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greatvovan commented Nov 5, 2020

Uh oh!

dimaqq commented Nov 5, 2020

Uh oh!

github-actionsBot commented Dec 16, 2020

Uh oh!

dimaqq commented Dec 16, 2020

Uh oh!

greatvovan commented Dec 16, 2020

Uh oh!

greatvovan commented Dec 18, 2020

Uh oh!

greatvovan commented Dec 30, 2020

Uh oh!

willingc commented Jan 5, 2021

Uh oh!

Uh oh!

bedevere-bot commented Jan 5, 2021

Uh oh!

greatvovan commented Jan 5, 2021

Uh oh!

greatvovan commented Jan 5, 2021

Uh oh!

berkerpeksag commented Jan 5, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

berkerpeksag left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Jan 6, 2021

Uh oh!

miss-islington commented Jan 6, 2021

Uh oh!

bedevere-bot commented Jan 6, 2021

Uh oh!

berkerpeksag commented Jan 6, 2021

Uh oh!

greatvovan commented Jan 6, 2021

Uh oh!

greatvovan commented Jan 6, 2021

Uh oh!

berkerpeksag commented Jan 6, 2021

Uh oh!

bedevere-bot commented Jan 6, 2021

Uh oh!

greatvovan commented Jan 6, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

greatvovan commented Nov 5, 2020•
edited by bedevere-bot
Loading

berkerpeksag commented Jan 5, 2021•
edited
Loading

greatvovan commented Jan 6, 2021•
edited
Loading