Il giorno mer 23 gen 2019 alle ore 11:33 Ivan Kelly <iv...@apache.org>
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 boolean is whether the placement
> strictly conforms to the placement policy, so both could return.
>
> ```
> class PlacementResult<T> {
>     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 4.10.
I am open to adopt this solution as well.

I have already prepared the PR for the new 'Pair' but it will be
really easy to change again.
https://github.com/apache/bookkeeper/pull/1915

Enrico





> ```
>
> -Ivan
>
> On Tue, Jan 22, 2019 at 6:55 PM Enrico Olivelli <eolive...@gmail.com> wrote:
> >
> > Il mar 22 gen 2019, 18:38 Sijie Guo <guosi...@gmail.com> ha scritto:
> >
> > > On Tue, Jan 22, 2019 at 8:40 AM Enrico Olivelli <eolive...@gmail.com>
> > > 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 third party
> > > > classes in order to preserve compatibility with incompatible changes
> > > > of the third party library.
> > > >
> > > > We already had such problems in the past.
> > > >
> > > > I think the best way to address this problem is to introduce one
> > > > specific class in BookKeeper, maybe an inner class of
> > > > EnsemblePlacementPolicy.
> > > >
> > >
> > > why not just introduce a Pair like class in bookkeeper-common module?
> > > instead of an inner class for EnsemblePlacementPolicy.
> > >
> >
> > Works for me
> >
> > Enrico
> >
> > >
> > >
> > > >
> > > > If we agree on this solution I can send the patch, it is very
> > > > straightforward
> > > >
> > > > Regards
> > > > Enrico
> > > >
> > >
> > --
> >
> >
> > -- Enrico Olivelli

Reply via email to