Re: svn commit: r992041 - in /subversion/trunk/subversion/bindings/javahl: native/CreateJ.cpp src/org/apache/subversion/javahl/CommitItem.java src/org/tigris/subversion/javahl/CommitItem.java tests/org/apache/subversion/javahl/SVNTests.java

On Thu, Sep 2, 2010 at 2:04 PM, <hwright_at_apache.org> wrote:
> Author: hwright
> Date: Thu Sep  2 18:04:32 2010
> New Revision: 992041
>
> URL: http://svn.apache.org/viewvc?rev=992041&view=rev
> Log:
> JavaHL: Make a couple of URI fields of the public java.net.URI type.

I don't agree with this at all. I hope you do not plan to run through
the API and make more of this.

In this case, there is zero benefit to doing this and I do not have
any confidence you have done the investigation needed to know if there
are any detriments. Do you KNOW that this class will accept all
possible values SVN will give it?

That said, this class only provides data from the API to the caller.
The caller is going to prefer String. Why make them run URI.toString?

The only place where stuff like this would make sense would be on
Input to our API. So that users knew what they were expected to
provide. As I said on IRC, I think it would be a mistake to do this.
String is easier. In Subclipse we use File or URL in our API, and it
is a pain to maintain. I wish we had just used String like JavaHL.

I think you should revert this.

-- 
Thanks
Mark Phippard
http://markphip.blogspot.com/