On 06/11/2013 21:37, Phil Steitz wrote:
> My +1 below stands, but here is something for the RM (Mark) and
> others to consider.
>
> Following the directions on the main site page, I realized that the
> change in r 1537253 to break the cyclic dependency between .pool2
> and .pool2.impl made my instructions incorrect. I did not fully get
> the implications of this change when it was committed and I failed
> to make the appropriate change to the site doc. I can update the
> site post-release no problem; but I wonder now if we might want to
> consider moving DefaultPooledObject up to .pool2 so that the simple
> (now incomplete) approach documented on the main page can work.
> Otherwise, you always have to add the wrap implementation, which I
> bet in the vast majority of cases will end up just being what
> makeObject did before the change:
>
> @Override
> public PooledObject<Foo> wrap(Foo foo) {
> return new DefaultPooledObject<Foo>(foo);
> }
>
> I get that DefaultPooledObject is an impl kind of thing; but so
> actually is BasePooledObjectFactory.
>
> Apologies - once again - for not having noticed this when reviewing
> the commit and RCs. I can live with it the way it is and as I said
> the site doc can be fixed post-release, so I am OK moving forward.
> Just wanted to do a little gut check before we pour the cement on
> this setup.
I think there are good arguments for both positions that are pretty
evenly matched. Having opted for one arrangement, I don't see a strong
enough argument to change it.
The o.a.c.pool2 package currently only contains interfaces, abstract
classes and a utility class. There are no concrete implementations.
Moving DefaultPooledObject would change that. Equally, I can see how
moving it would make things simpler but I did deliberately opt to leave
it in o.a.c.pool2.impl
I'm fine with fixing the docs post release.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
