On Mon, Jun 11, 2012 at 6:58 AM, Hyrum K Wright <hyrum.wri...@wandisco.com>wrote:
> On Mon, Jun 11, 2012 at 12:56 PM, 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. > > > > 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. > > I did not realize that I have done so, let me review and correct. Thank you, Vladimir > -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/ >
