On Fri, Dec 25, 2020 at 12:00 PM Daniel Shahaf <d...@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
