On Mar 8, 2010, at 10:04 AM, Mark Phippard wrote: > On Fri, Feb 12, 2010 at 12:13 PM, Hyrum K. Wright > <hyrum_wri...@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
