Skip to content

bpo-40956: Convert _sqlite3.Connection to Argument Clinic #23057

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

Closed

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Oct 31, 2020

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Oct 31, 2020

FYI, @corona10, this is the largest part of bpo-40956, and I see that it might be too much for one review. Let me know if I should break this PR up in multiple parts.

@corona10 corona10 self-requested a review October 31, 2020 15:07
@corona10 corona10 requested a review from vstinner November 11, 2020 15:14
@corona10
Copy link
Member

@vstinner cc @erlend-aasland
I am quite busy until next week, Can you please take a look instead of me? ;)

@erlend-aasland
Copy link
Contributor Author

@vstinner cc @erlend-aasland
I am quite busy until next week, Can you please take a look instead of me? ;)

Let me know if I should split this up; excluding the clinic code, the short stat is 3 files changed, 292 insertions(+), 210 deletions(-). I'll be happy to split it up, if that can help.

@erlend-aasland
Copy link
Contributor Author

All right, how about this: I'll split this up in two parts:

  1. All the straightforward methods
  2. Connection.execute*() and Connection.backup(), which require small modifications in addition to AC

The former, with the larger line count, should be the easiest to review. The latter may end up with adjustments; I may not have chosen the best way to resolve the issues encountered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants