Sending subversion/bindings/swig/include/svn_types.swg
Sending subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
Sending subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
Sending subversion/bindings/swig/svn_fs.i
Sending subversion/bindings/swig/svn_repos.i
Transmitting file data .....done
Committing transaction...
Committed revision 1880967.
I made only minor comment/formatting changes.
On Sun, Aug 2, 2020 at 9:36 AM Yasuhito FUTATSUKI <futat...@yf.bsdclub.org>
wrote:
> On 2020/07/31 10:19, Yasuhito FUTATSUKI wrote:
> > On 2020/07/30 22:20, C. Michael Pilato wrote:
> >> The patch seems to work pretty well (for svn_repos_fs_commit_txn), but
> >> there are two issues I have with it.
> >>
> >> First, unless I'm mistaken, there's no way to get the conflict_p back as
> >> part of the 2-tuple return value -- if there's a conflict,
> >> svn.repos.fs_commit_txn() is going to raise an Exception. So we're
> >> basically returning a 2-tuple whose first value is always None. We
> might
> >> as well preserve the current behavior and return the newly committed
> >> revision as a singleton. (Which also means we don't break compatibility
> >> with prior versions.)
> >
> > How do you think about this issue on svn.fs.commit_txn()? It is also
> > always (None, new_rev) if it doesn't raise Exception, since r845217,
> > Feb 2003. I also don't like this current behavor of svn.fs.commit_txn(),
> > however, I took priority of compatibility. (Though I think no one
> > ever used this Python wrapper function in practical program....)
> >
> > Putting aside this compatibility issue, my preference is:
> >
> > if error is not occured:
> > if *conflict_p is NULL:
> > return *new_rev as a singletone
> > else:
> > # a foolproof. we can detect an internal error by doing this.
> return 2-tuple (*conflict_p, *new_rev)
> > else:
> > # error is occred
> > raise exception with values of *conflict_p and *new_rev in the
> exception
> >
> > So I prefer to make this change and announce it. Note that we have
> > an option that we'll change behavor only in Python 3 with preserve
> > the behavor in Python 2.
>
> I've revised the patch, with this applying to both of Python 2 and Python
> 3.
>
> >> Also, I don't love that we've used "conflict_p" and "new_rev" as the
> >> variables names within the SubversionException. Anything we do here
> will
> >> be non-obvious and require documentation, so I'd rather give these
> things
> >> meaningful names ("conflict_path" and "new_revision"). But as a
> concept,
> >> I'd say this part works well.
> >
> > Using argment names of C API have a merit that if we will implement such
> > special wrappers requires attirbutes of SubversionException, we just say
> > implement it, without attributes names. Although my patch uses
> > "conflict_p" and "new_rev" as a C string literal, it can be "$1_name"
> > and "$2_name" (in the case of svn.fs.commit_txn).
> >
> > Of course, as there is no typemaps to be applied to multiple C APIs,
> > we should implement them one by one, so its trouble will be only a bit
> > that even we will name new attribute.
> In this patch, I used "$1_name" yet.
>
> Cheers,
> --
> Yasuhito FUTATSUKI <futat...@yf.bsclub.org>
>
