db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David W. Van Couvering" <David.Vancouver...@Sun.COM>
Subject Re: [jira] Updated: (DERBY-289) Enable code sharing between Derby client and engine
Date Mon, 21 Nov 2005 20:52:49 GMT
Thanks, Kathey, some responses below

Kathey Marsden wrote:
> David Van Couvering (JIRA) wrote:
> 
> 
>> 
>>Hi, all.  Here is the proposed patch that provides the framework for code sharing.
 I was thinking folks could look at it and discuss, and then once issues have (hopefully)
been worked out, we can have a vote.
>> 
>>
>>
> 
> Thanks for posting this patch. It helped me better understand the
> framework being proposed.l
> I looked at this perspective from the impact to existing users and risk
> of regression due to client/server jar incompatibilities.
> 
> Comments on the framework
> 
> I think that this framework seems perhaps workable for very stable and
> well established interfaces such as SanityManager.   I think it might be
> good to have the initial code sharing patch be just stable interfaces 
> like this and not include the message sharing which presents different
> issues than just the java code.

Yes, I agree

> 
> As I mentioned before,  I think for the messages this framework is not
> acceptable. New/changed  messages should not cause shadowing and require
> hasFeature() branches.  One of the other mechanisms discussed to
> separate the resources or load from multiple resources seems more
> appropriate.  I did not look closely at the message implementation after
> I decided that this is not really acceptable. 

OK

> 
> 
> For evolving interfaces or volatile resources the framework creates risk
> because.
> 
> 1) There is a fairly high chance of regression in compatibility because
> the coding requirements for compatibility are not that intuitive and are
> not caught by build or normal testing.  Also regressions in this area by
> definition will take a long  time to appear in normal operation.
> 2)Adding/changing new shared methods will mean adding a hasFeature()
> branch but the old code path has to stay in place for backward
> compatibility, making code messy over time and hard to cover.
> 3) The infrastructure itself won't get too much of a workout for a long
> time as it won't be needed until after 10.2
> 
> These risks I do not think are showstoppers, but I do think that care
> should be taken when migrating code to the common area to make sure it
> is well established and stabilized on either server or client before
> being moved. 
>

I agree.  As I mentioned, I am looking at a way to avoid the 
compatibility requirements.  If I am successful, I hope these issues 
will go away and it will be easier and less error-prone to have shared code.

> 
> Patch comments.
> 
> I think it would be good to include one specific example feature to the
> code that shows how the hasFeature() code branches work for folks adding
> features or messages and talk about it a bit in package.html.
> 

OK

> I think the @author tags are not supposed to be in the files from what I
> have heard on the list.

OK

> 
> I don't have any other specific implementation comments beyond what Dan
> and Deepa have mentioned already.
> 
> 
> Tests
> 
> I don't see what suite the test was added to.

I didn't add it to a suite because it can't run with the Derby jar 
files, it can only run with the classes directory (I haven't figured out 
a way to find a jar within a jar).  I think I mentioned this in the 
comments.

> 
> While the tests seem to test some of the assumptions made by the
> framework they don't seem to test the framework itself or cover the new
> common code.  Again I think a solid example (if only a contrived one),
> and a test path that goes through it would be better if possible.
>

OK

> I seem to recall some sort of problem in the build if the package and
> directory names don't match as  for sc.newClass etc. Maybe that is not
> the case for tests but I thought maybe I should mention it.

OK, I'll look into that.  The build seemed to work fine for me.


David

> 
> Thanks
> 
> Kathey
> 
> 
> 

Mime
View raw message