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
Received on 2020-12-28 11:11:25 CET