On 3/10/06, Sandy McArthur <[EMAIL PROTECTED]> wrote: > On 3/10/06, Phil Steitz <[EMAIL PROTECTED]> wrote: > > Any comments on this? > > http://issues.apache.org/bugzilla/show_bug.cgi?id=38073 > > > > As I see it, there are two decent alternatives here. The one I first > > suggested in the report is too ugly. > > > > (i) Make the (incorrectly) implemenented getReference method in > > InstanceKeyDataSource throw, as described in the last comment on the > > report, and override with correct impls in the concrete subclasses. > > > > (ii) Make the (minor) backward-incompatible change to make the > > offending method in InstanceKeyDataSource abstract, as suggested in > > the original report, again providing correct impls in the subclasses. > > > > I think (ii) is cleaner (what should have been done in the first > > place) but is technically a backward incompatible change. Users who > > have subclassed will be forced to implement. > > > > Interested parties, please weigh in with opinions or better ideas > > about how to resolve this. > > (Standard disclaimer about not being familiar with Dbcp yet.) > > For a bug fix release it seems to me that you should override > getReference() in PerUserPoolDataSource and SharedPoolDataSource. And > document in InstanceKeyDataSource.getReference() that it will be going > away in the next major release. > > Also, it seems to me that changing > InstanceKeyDataSource.getReference() for the ref var to: > Reference ref = new Reference(getClass().getName(), > getClass().getName() + "Factory", null); > > would work better. It's fragile but I don't think it would break anything new. > > > Also scream now if adding test > > dependencies on the (ibiblio published) tomcat naming jars is not > > acceptable. > > For building and testing only right? Not for deployment of Dbcp?
Yes, just a test dependency. Not required at all for dist or runtime. Phil > > -- > Sandy McArthur > > "He who dares not offend cannot be honest." > - Thomas Paine > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
