Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Received-SPF: pass (asf.osuosl.org: local policy)
Date: Mon, 21 Nov 2005 12:52:49 -0800
From: "David W. Van Couvering" <David.Vancouvering@Sun.COM>
Subject: Re: [jira] Updated: (DERBY-289) Enable code sharing between Derby
 client and engine
In-reply-to: <43821580.9080901@sbcglobal.net>
To: derby-dev@db.apache.org
Message-id: <438233A1.90100@sun.com>
MIME-version: 1.0
Content-type: multipart/mixed; boundary="Boundary_(ID_WDC0SQscqZl2Z9D0RxvLhA)"
User-Agent: Mozilla Thunderbird 1.0.7 (Windows/20050923)
References: <922737490.1131395000353.JavaMail.jira@ajax.apache.org>
 <43821580.9080901@sbcglobal.net>

This is a multi-part message in MIME format.

--Boundary_(ID_WDC0SQscqZl2Z9D0RxvLhA)
Content-type: text/plain; charset=UTF-8; format=flowed
Content-transfer-encoding: 7BIT

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
> 
> 
> 

--Boundary_(ID_WDC0SQscqZl2Z9D0RxvLhA)
Content-type: text/x-vcard; charset=utf-8; name=david.vancouvering.vcf
Content-transfer-encoding: BASE64
Content-disposition: attachment; filename=david.vancouvering.vcf

YmVnaW46dmNhcmQNCmZuOkRhdmlkIFcgVmFuIENvdXZlcmluZw0KbjpWYW4gQ291
dmVyaW5nO0RhdmlkIFcNCm9yZzpTdW4gTWljcm9zeXN0ZW1zLCBJbmMuO0RhdGFi
YXNlIFRlY2hub2xvZ3kgR3JvdXANCmVtYWlsO2ludGVybmV0OmRhdmlkLnZhbmNv
dXZlcmluZ0BzdW4uY29tDQp0aXRsZTpTZW5pb3IgU3RhZmYgU29mdHdhcmUgRW5n
aW5lZXINCnRlbDt3b3JrOjUxMC01NTAtNjgxOQ0KdGVsO2NlbGw6NTEwLTY4NC03
MjgxDQp4LW1vemlsbGEtaHRtbDpUUlVFDQp2ZXJzaW9uOjIuMQ0KZW5kOnZjYXJk
DQoNCg==

--Boundary_(ID_WDC0SQscqZl2Z9D0RxvLhA)--