[PATCH 1 of 8] bdiff: add _version to help detect breaking binary changes
Jun Wu
quark at fb.com
Tue May 2 20:56:31 EDT 2017
Excerpts from Gregory Szorc's message of 2017-05-02 17:41:21 -0700:
> I like the spirit of this series.
>
> Have you given any consideration to:
>
> * Sharing a version number across all modules? That will make the code
> easier at the expense of recompiling all C modules when not strictly
> necessary. Since the base rate of changing C modules is low and everything
> but zstd compiles almost instantaneously, one can make an argument that
> simplicity is sufficient.
While I think that also works, I think the benefit (less repetitive code) is
not that significant given the fact that I think CPython C code is already
cumbersome (look at those PyInits), so I won't optimize code length for them
as much as I do for non-Python C code.
I also slightly prefer the explicit way of saying which modules we are
checking and their expected versions in Patch 8.
> * A shared function or macro for calling PyModule_AddIntConstant(). It
> feels like a bit of redundant code across all the modules.
Suppose we have a .h file for that feature, then .c file still need roughly
same number of lines of change:
+#include "...."
+PyObject *m;
+m = PyModule_Create(&bdiff_module); // Python 2
+define_version(m) // Python 2
+define_version(m) // Python 3
Maybe we can unified the Py2 vs Py3 module definition. But that seems to be
another series which should not block this one.
More information about the Mercurial-devel
mailing list