On Apr 21, 2013, at 11:09 AM, Ulf Zibis wrote: > Minor nits: > DriverManager line 349: I would break the line right after the opening > parenthesis.
I did not change this as other methods break after the 1st parameter (getConnection is one example) so I would prefer to keep it consistent (or as much as it can be) > DriverManager line 355: missing space after comma. fixed this. thank for this Best Lance > > -Ulf > > Am 21.04.2013 13:45, schrieb Lance Andersen - Oracle: >> Thank you for the feedback Alan, >> >> Please see below and the webrev >> http://cr.openjdk.java.net/~lancea/8010416/webrev.02/ >> On Apr 21, 2013, at 4:34 AM, Alan Bateman wrote: >> >>> On 19/04/2013 18:34, Lance Andersen - Oracle wrote: >>>> Hi, >>>> >>>> We have been asked by a few JDBC driver vendors to allow a JDBC driver >>>> to be notified when/if it was deregistered via >>>> DriverManager.deregisterDriver if desired. >>>> >>>> >>>> The webrev can be found at >>>> http://cr.openjdk.java.net/~lancea/8010416/webrev.01 >>>> >>> This looks much better than the original proposal, it would have been just >>> too problematic to have Driver define a deregister method. Also the >>> proposed wording to specify Driver-implementation specific behavior when >>> the Driver or Connections is in use, or subsequent use, addresses the >>> points that I brought up in the original thread here (thanks!). >>> >>> Driver >>> >>> - {@linkplain DriverManager.deregister} -> I assume this should >>> DriverManagert#deregisterDriver >>> >>> - minor alignment issue with the <p> tag. >> Fixed >>> >>> DriverManager >>> >>> - one point that isn't covered in the spec is whether the DriverAction's >>> deregister is invoked before or after it is deregistered. This distinction >>> is probably only interesting for the case that the deregister method fails >>> with an error/exception but it's not clear if the driver is still >>> registered in that case. For completeness then the spec should probably say >>> that any error/runtime exception is propagated to the caller of >>> deregisterDriver. >>> >>> - could the new (and the original) registerDriver methods specify the >>> behavior for the case that the Driver is already registered? This brings up >>> the question as to whether the DriverAction is overridden if already >>> registered. >> clarified >>> - the @param alignment is inconsistent in the new deregisterDriver. >> done >>> - the re-wording of the original deregisterDriver looks much better. Minor >>> nit: "was null" -> "is null". >> done >>> - DriverInfo - would be cleaner to extend the constructor to take the >>> DriverAction. Also an action() accessor would make the usage a bit cleaner >>> too. >> done >> >> Best >> Lance >>> >>> That's all I have for now. >>> >>> -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 >> >
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com