Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

On Mon, Jun 11, 2012 at 12:56 PM, Hyrum K Wright
<hyrum.wright_at_wandisco.com> wrote:
> On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker <vmpn_at_hitechman.com> wrote:
>> Greetings,
>>
>> This patch signifies a point post which I can merge the existing logic on
>> the
>> javahl-ra branch with starting parts of my proposed enhancements. So I hope
>> people still have some patience left to review them.
>>
>> This patch moves parts of client context that are shared with ra context
>> into
>> common classes.  As I cannot get svn to generate patches that deal with
>> copies
>> of existing file, before applying below the following svn copy command have
>> to
>> be issued:
>>
>> svn cp
>> subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
>> subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
>> svn cp subversion/bindings/javahl/native/ClientContext.h
>> subversion/bindings/javahl/native/CommonContext.h
>> svn cp subversion/bindings/javahl/native/ClientContext.cpp
>> subversion/bindings/javahl/native/CommonContext.cpp
>>
>>
>> Also please note that java part of this patch is already applied in r1343452
>> & r1347345
>> on javahl-ra branch, due to my earlier mistake.
>
> If this work is specific to the things happening on the branch, I
> think you're fine committing it directly there.
>
> One question, though.  The ClientContext object directly mirrors the
> svn_client_context_t struct in the C layer (the analogs are probably
> obvious).  To my knowledge, we don't have a similar struct for the C
> ra layer.  I'm just a little confused why we need to break this clear
> correlation with the C layer in Java, when we don't need to in C.
> I'll need to think on this a bit...

Oh, one other thing: we like to separate white-space and functional
changes into separate commits. I noticed a large part of your patch
is reindenting various files to conform with the rest of the codebase
(good!), and I'm wondering if we could apply that portion independent
of the others.

-Hyrum

>>
>> Thank you for your help,
>>
>> Vladimir
>>
>> [[[
>> JavaHL: Factor out common context to be shared between SVNClient and SVNRa
>> classes
>>
>> [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
>>
>> * CommonContext.java,
>>   SVNClient.java
>>   (ClientContext): Move to progress listener into CommonContext
>>
>> [ in subversion/bindings/javahl/native ]
>>
>> * CommonContext.cpp,
>>   CommonContext.h,
>>   ClientContext.cpp,
>>   ClientContext.h
>>   (username, password, getConfigDirectory, setConfigDirectory, setPrompt,
>>    cancelOperation, progress): Move from ClientContext to CommonContext
>>
>> * CommonContext.cpp,
>>   CommonContext.h
>>   (attachJavaObject): New function to hold common logic of attaching to the
>>     java CommonContext class used for callbacks
>>   (getConfigData, getAuthBaton): Split getContext into separate
>> configuration
>>     data setup and authentication data setup to better reflect their
>> different life cycles
>>   (getClientName): New function providing client name to be used in
>> callbacks
>>
>> * ClientContext.cpp,
>>   ClientContext.h
>>   (ClientContext, getContext): Use the factored out CommonContext member
>>     variables and functions
>> ]]]
>>
>
>
>
> --
>
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com/

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/