On Mar 8, 2010, at 10:04 AM, Mark Phippard wrote:
> On Fri, Feb 12, 2010 at 12:13 PM, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> Prologue: I'm pretty indifferent on the structure of the JavaHL packages and such. I just help >maintain the JNI glue, so I've approached these changes from a JNI/C++ perspective.
>>
>> On Feb 12, 2010, at 8:08 AM, Mark Phippard wrote:
>>
>>> Renaming the JavaHL classes and providing compatibility wrappers for
>>> the old classes has allowed us to make some changes in the new
>>> classes, such as dropping deprecated classes and renaming things etc.
>>> I took the opportunity to ask the SVNKit developers if there was
>>> anything more we could do to help them. They provide a pure-Java
>>> implementation of the JavaHL interface and I know they ran into a few
>>> areas in our code where we did things that made this difficult.
>>>
>>> They provided me the attached patch and accompanying explanation of
>>> the changes. It all makes sense to me, but I thought I would run it
>>> by the list. A lot of these items, such as creating an interface for
>>> SVNAdmin, are really obvious. Maybe the constructor change will be
>>> more controversial, but I think it reflects what needs to be done if
>>> the JavaHL interface is truly meant to be something that could have
>>> alternate implementations.
>>>
>>> Here is there explanation of the changes in the patch:
>>>
>>> 1. Changed all package local constructors to public ones. This will let
>>> SVNKit keep as much of the SVNKit-code out of the Subversion namespace
>>> as possible. Also, current version is not consistent in that aspect, for
>>> instance Status class constructor is public while Info2 class has
>>> package local one.
>>
>> I'm all for consistency, but the advantage of having private constructors is that we can still access them from JNI (which theoretically is the only thing using them), but don't have to maintain older versions of them for backward compat. If that theory isn't correct, and we chose to make them public, I suppose that's okay.
>
> I did not really process this comment as an objection last time so
> went ahead and committed.
Nope, this wasn't an objection, just a thought. Thanks for following through and committing these patches, (and helping us to think about the other issues).
> Anyone that wants to provide their own implementation of the JavaHL
> interface winds up having to use these classes because they are
> exposed via the interface or by other classes exposed by the
> interface. So what they effectively have to do is put code inside our
> package namespace so that they can access the constructor.
>
> If we have been changing these constructors in the past without
> versioning them, I do not know how they have been dealing with it. I
> had not thought of it and just assumed we have not really had to do
> this often.
>
> As I mentioned in the other email, I think a way to approach this
> would be to only publish interfaces as public API. The interfaces
> should not use any classes, just other interfaces. We could them keep
> our implementation of those interfaces private or at least marked in a
> package where we declare we are not going to maintain compat. Maybe
> we would also need a factory class somewhere in the public space. Not
> sure as I have not thought this completely through.
I've followed up on these comments somewhat in the other thread.
-Hyrum
Received on 2010-03-08 17:09:48 CET