Yasuhito FUTATSUKI wrote on Sat, 26 Dec 2020 11:02 +00:00: > On 2020/12/26 3:38, Nathan Hartman wrote: > > > But the solution you suggest above, adding a new_rev attribute to the > > exception and leaving the return value as it was, sounds like a good way to > > avoid the incompatibility and also avoid the API bump. > > I had written such a patch on typemaps at first because it was straight > forward. Although I removed it after r1880967 is commited, I can write > such a code again and I'll do if it is needed. > > Alternately, here is an easier way to do it. The patch below is a code > that replace wrapper function entity on top of the svn.fs module > (not tested, and more appropriate comment is needed in the code). > > [[[ > Index: subversion/bindings/swig/python/svn/fs.py > =================================================================== > --- subversion/bindings/swig/python/svn/fs.py (revision 1884802) > +++ subversion/bindings/swig/python/svn/fs.py (working copy) > @@ -24,6 +24,14 @@ > ###################################################################### > > from libsvn.fs import * > + > +# For API compatibility we should replace wrapper function entity before > +# adding alternative names. > +_svn_fs_commit_txn = svn_fs_commit_txn > +def svn_fs_commit_txn(*args) -> "char const **, svn_revnum_t *": > + r"""svn_fs_commit_txn(svn_fs_txn_t txn, apr_pool_t pool) -> > svn_error_t""" > + return None, _svn_fs_commit_txn(*args) > + > from svn.core import _unprefix_names, Pool, _as_list > _unprefix_names(locals(), 'svn_fs_') > _unprefix_names(locals(), 'SVN_FS_') > > ]]] > > Of course, we can use svn_fs_commit_txn2 instead of _svn_fs_commit_txn > in above patch, then it will satisfy the Daniel's proposal.
I agree with Nathan that it'd be confusing for a Python svn_fs_commit_txn2() to exist when a C function of the same name does not, even if the C API skips a revv number in consideration of swig-py. So, if we'd like to introduce a simplified API, I'd vote to name it something else. Cheers, Daniel
