bpo-1635741: Port _collections module to multiphase initialization. #19074
Uh oh!
There was an error while loading. Please reload this page.
Conversation
For the record, I will work on a new helper function after this PR is landed if this work is worth to do. |
corona10 commented Mar 19, 2020
@shihai1991 Please take a look :) |
vstinner left a comment
vstinner left a comment There was a problem hiding this comment.
LGTM apart "m" variable name which looks too short to me ;-)
| if (PyType_Ready(type) <0){ | ||
| return-1; | ||
| } | ||
| constchar*name=_PyType_Name(type); |
There was a problem hiding this comment.
FYI I checked manually that the actual type name and _PyType_Name() gives the same name than before this change.
There was a problem hiding this comment.
I did same thing also :) Thank you for the re-checking!
Modules/_collectionsmodule.c Outdated
| } | ||
| constchar*name=_PyType_Name(type); | ||
| Py_INCREF(type); | ||
| if (PyModule_AddObject(m, name, (PyObject*)type) <0){ |
There was a problem hiding this comment.
Checking for PyModule_AddObject() error is a good thing ;-)
Modules/_collectionsmodule.c Outdated
| }; | ||
| staticint | ||
| collections_exec(PyObject*m){ |
There was a problem hiding this comment.
nitpick: would you mind to rename it to "mod" or "module"?
There was a problem hiding this comment.
module might be great! Thanks for the suggestion
shihai1991 commented Mar 19, 2020
oh, on~ my network crashed at this point :( |
| defdict_type.tp_base=&PyDict_Type; | ||
| for (inti=0; typelist[i] !=NULL; i++){ |
shihai1991 left a comment
shihai1991 left a comment There was a problem hiding this comment.
I like the improvment of typelist.
LGTM :)
corona10 commented Mar 19, 2020
Thanks for the reviews :) |
5 participants
https://bugs.python.org/issue1635741