Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
corona10 merged 5 commits into from
Oct 31, 2020
Merged

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

corona10 merged 5 commits into from
Oct 31, 2020

Conversation

@erlend-aasland
Copy link
Contributor

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

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
Copy link
ContributorAuthor

erlend-aasland commented Oct 11, 2020

@vstinner, would you mind reviewing this?

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Oct 15, 2020

@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
Copy link
Member

corona10 commented Oct 20, 2020

cc @shihai1991

@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Oct 22, 2020

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

@corona10
Copy link
Member

corona10 commented Oct 23, 2020

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
Copy link
ContributorAuthor

erlend-aasland commented Oct 23, 2020

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
Copy link
Member

@corona10corona10 left a comment

Choose a reason for hiding this comment

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

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

@bedevere-bot
Copy link

bedevere-bot commented Oct 28, 2020

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 Aaslandand 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
Copy link
ContributorAuthor

erlend-aasland commented Oct 30, 2020

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Oct 30, 2020

Thanks for making the requested changes!

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

corona10
corona10 approved these changes Oct 31, 2020
View reviewed changes
Copy link
Member

@corona10corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

Thank you for working on this ;)

@corona10corona10 merged commit 7d21027 into python:masterOct 31, 2020
@erlend-aasland
Copy link
ContributorAuthor

erlend-aasland commented Oct 31, 2020

Thank you for working on this ;)

Thank you and likewise! :)

@erlend-aaslanderlend-aasland deleted the bpo-40956/part1-mod branch October 31, 2020 09:41
@erlend-aaslanderlend-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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10corona10corona10 approved these changes

@berkerpeksagberkerpeksagAwaiting requested review from berkerpeksag

@vstinnervstinnerAwaiting 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