◐ Shell
reader mode source ↗
Skip to content

bpo-40956: Convert _sqlite3 module level functions to Argument Clinic#22484

Merged
corona10 merged 5 commits into
python:masterfrom
erlend-aasland:bpo-40956/part1-mod
Oct 31, 2020
Merged

bpo-40956: Convert _sqlite3 module level functions to Argument Clinic#22484
corona10 merged 5 commits into
python:masterfrom
erlend-aasland:bpo-40956/part1-mod

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Oct 1, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Part 1 of 5, @vstinner.
Ref #20826

Notes:

  • _sqlite3.connect will be converted in the final PR
  • sqlite3.adapt is moved from microprotocols.c to module.c

https://bugs.python.org/issue40956

Convert _sqlite3 module level functions, except connect, to Argument … …
39df100 
…Clinic

Note: Moves sqlite3.adapt from microprotocols.c to module.c
@erlend-aasland

erlend-aasland commented Oct 11, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner, would you mind reviewing this?

@erlend-aasland

erlend-aasland commented Oct 15, 2020

Copy link
Copy Markdown
Contributor Author

@corona10 Would you you mind reviewing this as well, if you've got time for it? It will be a series of five (or six) PR's.

@corona10 corona10 requested review from corona10 and vstinner October 19, 2020 17:13
@corona10

corona10 commented Oct 20, 2020

Copy link
Copy Markdown
Member

cc @shihai1991

@erlend-aasland

erlend-aasland commented Oct 22, 2020

Copy link
Copy Markdown
Contributor Author

Would it be preferable to split this PR in smaller chunks? One PR for each method converted?

@corona10

corona10 commented Oct 23, 2020

Copy link
Copy Markdown
Member

Would it be preferable to split this PR in smaller chunks? One PR for each method converted?

Oops sorry, I will review this PR until next week, sorry for the delay :(

@erlend-aasland

erlend-aasland commented Oct 23, 2020

Copy link
Copy Markdown
Contributor Author

Oops sorry, I will review this PR until next week, sorry for the delay :(

No problem, @corona10, and thank you for taking this! I hope I'm not too eager/impatient.

corona10
corona10 requested changes Oct 28, 2020
View reviewed changes

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

I've checked some APIs.
Please check other things also ;)

@bedevere-bot

bedevere-bot commented Oct 28, 2020

Copy link
Copy Markdown

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.

Erlend Egeberg Aasland and others added 3 commits October 28, 2020 21:04
@corona10
complete_statement() should also accept 'statement' as a keyword argu… …
fab488b 
…ment

Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@corona10
Ditto for enable_shared_cache() …
7ba5f23 
Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@corona10
Keep current argument naming …
81fd61c 
Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@erlend-aasland

erlend-aasland commented Oct 30, 2020

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented Oct 30, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@corona10: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from corona10 October 30, 2020 06:55
corona10
corona10 approved these changes Oct 31, 2020
View reviewed changes

@corona10 corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

lgtm

Thank you for working on this ;)

@corona10 corona10 merged commit 7d21027 into python:master Oct 31, 2020
@erlend-aasland

erlend-aasland commented Oct 31, 2020

Copy link
Copy Markdown
Contributor Author

Thank you for working on this ;)

Thank you and likewise! :)

@erlend-aasland erlend-aasland deleted the bpo-40956/part1-mod branch October 31, 2020 09:41
@erlend-aasland erlend-aasland mentioned this pull request Dec 30, 2020
Merged
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@adorilson
bpo-40956: Convert _sqlite3 module level functions to Argument Clinic (… …
f852401 
…pythonGH-22484)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

@vstinner vstinner Awaiting requested review from vstinner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@erlend-aasland @corona10 @bedevere-bot @taleinat @the-knights-who-say-ni