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
begin:vcard
fn:David W Van Couvering
n:Van Couvering;David W
org:Sun Microsystems, Inc.;Database Technology Group
email;internet:[EMAIL PROTECTED]
title:Senior Staff Software Engineer
tel;work:510-550-6819
tel;cell:510-684-7281
x-mozilla-html:TRUE
version:2.1
end:vcard
