On Mon, Jun 11, 2012 at 9:09 AM, Hyrum K Wright <hyrum.wri...@wandisco.com>wrote:
> On Mon, Jun 11, 2012 at 2:58 PM, Vladimir Berezniker <v...@hitechman.com> > wrote: > > > > > > On Mon, Jun 11, 2012 at 6:56 AM, Hyrum K Wright < > hyrum.wri...@wandisco.com> > > wrote: > >> > >> On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker <v...@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? > How about RASharedContext? It is RA layer data that is used across client and ra functions. Also, I have not correctly described the role of the svn_commit_callback2_t. While its JNI implementation is shared between client and ra layers. It is not part of the svn_client_ctx_t nor svn_ra_callbacks2_t nor their JNI counterparts. Rather, it is passed directly to the svn_client_commit6 and svn_ra_get_commit_editor3 functions. Sorry for the confusion, my brain was still thinking of patch series, where one of the patches factors this out to common code, when I mentioned it. Vladimir > > -Hyrum > > > > -- > > uberSVN: Apache Subversion Made Easy > http://www.uberSVN.com/ >
