On Fri, Dec 25, 2020 at 12:00 PM Daniel Shahaf <d.s_at_daniel.shahaf.name>
wrote:
>
> I may be more than a bit late for this, but can we avoid the
> incompatibility in the first place? I.e., avoid requiring API users to
> check svn.core.SVN_VER_MINOR before interpreting an API's return value?
> Adding a new_rev attribute to the exception is a welcome change — it
> addresses Nathan's point (1) — but I question whether changing «return
> (None, foo)» to «return foo» (plus or minus a singleton tuple around it)
> justifies breaking compatibility. This change sounds like it should be
> part of a proper API bump, namely, provide a svn.fs.commit_txn2(),
> keeping svn.fs.commit_txn() as-is.
>
> (Implementation-wise, it may be easier to implement svn.fs.commit_txn2()
> as a wrapper around .commit_txn() than the other way around. When we
> next revv the C API, we can skip a revv number in consideration of the
> bindings, and call the revved API svn_fs_commit_txn3(), deprecating
> svn_fs_commit_txn().)
I thought about suggesting an API bump for the Python wrapper but I didn't
suggest it because I thought it might be confusing to have
svn.fs.commit_txn2() be a wrapper for svn_fs_commit_txn(), especially if
there's ever a svn_fs_commit_txn2().
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.
Cheers,
Nathan
Received on 2020-12-25 19:38:40 CET