Thank you for the comments Alan On Sep 6, 2012, at 9:00 AM, Alan Bateman wrote:
> On 06/09/2012 13:40, Lance Andersen - Oracle wrote: >> Here is the updated webrev >> http://cr.openjdk.java.net/~lancea/7192302/webrev.01 >> >> I know there is more clean-up that can be done to remove other Rave added >> code (such as the removal of set/getPreparedStatement/Connection/ResultSet), >> I want to keep the focus to just removing PropertyChangeSupport. SQE and >> RowSet TCKs continue to pass with these changes. >> > Your previous mail uses the word "nuke", I was thinking the same thing :-) > > The latest webrev looks okay except that in one of the constructors you have > removed a call to ensure that the connection is established, I'm not sure > about the significance of that. This is not needed here and given I have already tested with this removed, I figured I would OK to keep this as part of the change > > I also see there are a couple of residual references to "Rave" that can > probably be pulled, these seem to be related to the PropertyChangeListener > support. I left those in as a reminder to go back as part of the rest of the Rave clean-up. I would prefer to leave them for now and when I make another pass for Rave, I will get rid of them > > One method that looks like it could be removed too is setConcurrency but I > agree that keeping focused on just removing the beans dependency is right for > now. I had thought about that but I have to think about this one a bit more as the getConcurrency() is leveraging the value returned from the active ResultSet which is why I did not remove this at this time. In some cases, I must confess the original authors have left me scratching my head on some of this code but I want to be surgical in removing or addressing changes so that it would be easier to find any un-intended gotchas. Best Lance > > -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
