According to the new unit test, the attached patch works. Thoughts?
- Dan
On Fri, 17 Mar 2006, Tim Dionne wrote:
> Yes, makes good sense to me.
>
> +1
>
> On Fri, 2006-03-17 at 15:22 -0800, Daniel Rall wrote:
> > On Wed, 15 Mar 2006, Tim Dionne wrote:
> >
> > > On Wed, 2006-03-15 at 17:07 -0800, Daniel Rall wrote:
> > >
> > > > I have one main objection, which is easily rectifiable: this function
> > > > isn't specific to the client library, but part of the utility library
> > > > (declared in svn_path.h).
> > > >
> > > > AFAICT, there's only one other set of APIs on the SVNClient JNI
> > > > binding which doesn't belong to the client library:
> > > > getAdminDirectoryName() and setAdminDirectoryName().
> > > >
> > > > I'd prefer to create a separate JNI binding which exposes a native API
> > > > on a new class (maybe org.tigris.subversion.Path?). Thoughts?
> > >
> > > If you mean create a new binding, org.tigris.subversion.javahl.Path, and
> > > expose these 3 apis there, then I might suggest that the JNI binding be
> > > a little more generic.
> >
> > I was suggesting exposing only the new API there. It's not specific
> > to SVNClient (placing it only there would be somewhat arbitrary,
> > considering that we also have SVNAdmin).
> >
> > > The native apis that getAdminDirectoryName() and
> > > setAdminDirectoryName() call (svn_wc_get_adm_dir and
> > > svn_wc_set_adm_dir) are in libsvn_wc (svn_wc.h). Perhaps we should
> > > create a utility binding (maybe
> > > org.tigris.subversion.javahl.SVNUtility), and put these 3 utility
> > > apis in it.
> >
> > I worry that as more APIs are exposed through JavaHL
> > (e.g. libsvn_repos, libsvn_fs, etc.) that this utility class will
> > become more and more incohesive, as it becomes a dumping ground for
> > unrelated routines from various libraries.
> >
> > We can't easily "take away" APIs if we refactor'em into a separate
> > class, since doing so would break backwards compatibility. I'd like
> > to get this right in a forward-looking way now, rather than pay for it
> > later.
> >
--
Daniel Rall
- text/plain attachment: patch
- application/pgp-signature attachment: stored
Received on Tue Mar 21 03:43:01 2006