Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-23 Thread Enrico Olivelli
Il giorno mer 23 gen 2019 alle ore 11:48 Ivan Kelly ha scritto: > > > > ``` > > > class PlacementResult { > > > T result(); > > > boolean strictlyConformsToPolicy(); > > > } > > > > That was my first proposal and I like it. It is clearer and > > auto-documenting. > > > > Given that we are

Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-23 Thread Ivan Kelly
> > ``` > > class PlacementResult { > > T result(); > > boolean strictlyConformsToPolicy(); > > } > > That was my first proposal and I like it. It is clearer and auto-documenting. > > Given that we are changing EnsemblePlacementPolicy at every major > version, we can defer this refactor to

Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-23 Thread Enrico Olivelli
Il giorno mer 23 gen 2019 alle ore 11:33 Ivan Kelly ha scritto: > > There's no harm in having our own tuple implementation in common, but > in this instance we should encode more meaning into the returned > value. As it is, it's not even java documented. > But in both cases, it looks like the bool

Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-23 Thread Ivan Kelly
There's no harm in having our own tuple implementation in common, but in this instance we should encode more meaning into the returned value. As it is, it's not even java documented. But in both cases, it looks like the boolean is whether the placement strictly conforms to the placement policy, so

Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-22 Thread Enrico Olivelli
Il mar 22 gen 2019, 18:38 Sijie Guo ha scritto: > On Tue, Jan 22, 2019 at 8:40 AM Enrico Olivelli > wrote: > > > Hi all, > > while reviewing 4.9 release I found this problem around a change about > > EnsemblePlacementPolicy > > > > this is the issue > > https://github.com/apache/bookkeeper/issue

Re: EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-22 Thread Sijie Guo
On Tue, Jan 22, 2019 at 8:40 AM Enrico Olivelli wrote: > Hi all, > while reviewing 4.9 release I found this problem around a change about > EnsemblePlacementPolicy > > this is the issue > https://github.com/apache/bookkeeper/issues/1914 > > The problem is that in public API we should not use thir

EnsemblePlacementPolicy exposes third party API "Pair" from commons-lang3 in a public API

2019-01-22 Thread Enrico Olivelli
Hi all, while reviewing 4.9 release I found this problem around a change about EnsemblePlacementPolicy this is the issue https://github.com/apache/bookkeeper/issues/1914 The problem is that in public API we should not use third party classes in order to preserve compatibility with incompatible ch