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

On Mon, Jun 11, 2012 at 2:58 PM, Vladimir Berezniker <vmpn_at_hitechman.com> wrote:
>
>
> On Mon, Jun 11, 2012 at 6:56 AM, 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.
>
>
> Similar to the other patches so far, while the logic is used by RA
> layer. The re factoring is applicable to the trunk on its own. Though
> not as useful as it does not take care of the additional flexibility on
> its own. It does lean more towards RA specific enhancements than the
> other patches.
>
>>
>>
>> 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...
>
>
> The refactoring represents the common elements between
> the svn_client_ctx_t,
> svn_ra_callbacks2_t and svn_commit_callback2_t needed by the
> (svn_ra_get_commit_editor3) function. This overlap is demonstrated by the
> existing code in the (svn_client__open_ra_session_internal) function that
> goes
> between the client context and ra session. Specifically client and ra layers
> share
> the following objects:
>
>    * svn_auth_baton_t
>    * apr_hash_t (config) needed to create svn_auth_baton_t
>    * svn_ra_progress_notify_func_t
>
> Hope this helps,

It helps a lot! Should we then call the new object something a bit
more descriptive? Maybe CommitContext?

-Hyrum

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