Skip to content

bpo-30262: Deprecate sqlite Cache and Statement#1573

Closed
palaviv wants to merge 6 commits into
python:masterfrom
palaviv:dont-export-sqlite-cache-and-statement2
Closed

bpo-30262: Deprecate sqlite Cache and Statement#1573
palaviv wants to merge 6 commits into
python:masterfrom
palaviv:dont-export-sqlite-cache-and-statement2

Conversation

@palaviv
Copy link
Copy Markdown
Contributor

@palaviv palaviv commented May 13, 2017

@mention-bot
Copy link
Copy Markdown

@palaviv, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Yhg1s, @ghaering and @benjaminp to be potential reviewers.

Copy link
Copy Markdown
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

We need to add a NEWS entry and a release note in Doc/whatsnew/3.8.rst (see the "Deprecated" section at https://docs.python.org/3.8/whatsnew/3.8.html#deprecated)

Comment thread Lib/sqlite3/test/regression.py
Comment thread Modules/_sqlite/module.c Outdated
{
pysqlite_Cache *self;

/* print deprecation warning */
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.

Style nit: No need to add a comment for this.

Comment thread Modules/_sqlite/module.c Outdated

/* print deprecation warning */
if (PyErr_WarnEx(PyExc_DeprecationWarning,
"Cache object has been deprecated",
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.

Could you add "it won't be exposed in Python 3.9" or something similar as well?

Comment thread Modules/_sqlite/module.c Outdated
{
pysqlite_Statement *self;

/* print deprecation warning */
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.

See my comments on pysqlite_cache_deprecated_new().

Comment thread Modules/_sqlite/module.c Outdated
return (PyObject *)self;
}

/* classes for statement and cache deprecation */
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.

Style nit: This can comment can be deleted.

Comment thread Modules/_sqlite/module.c Outdated
Py_INCREF(&pysqlite_StatementType);
PyModule_AddObject(module, "Cache", (PyObject*) &pysqlite_CacheType);
Py_INCREF(&pysqlite_StatementDeprecatedType);
PyModule_AddObject(module, "Statement", (PyObject*)&pysqlite_StatementDeprecatedType);
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.

Style nit: Since we already modified this line, we can adda space after (PyObject*) to make the styling consistent.

@berkerpeksag
Copy link
Copy Markdown
Member

Oh, and please rebase dont-export-sqlite-cache-and-statement2 branch :) Thanks!

@palaviv palaviv force-pushed the dont-export-sqlite-cache-and-statement2 branch from 2f936ae to b37d55c Compare October 8, 2018 11:05
@matrixise
Copy link
Copy Markdown
Member

Hi @palaviv

Would you be interested to upgrade your PR to the last master?

Thank you

@palaviv
Copy link
Copy Markdown
Contributor Author

palaviv commented May 9, 2019

Not needed after #1440.

@palaviv palaviv closed this May 9, 2019
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.

6 participants