Hi Tom, I think these are all fair points. I was actually part of the group that decided that constructors should not be public. Since then I've noticed that many users find this limiting. I think we should make testability a key criteria for our APIs. Requiring a mocking library for data classes like this is suboptimal.
To understand the value of the package private constructors, I have some questions: 1. For the cases where we evolved the constructors, how much did we gain? 2. Would a deprecation + removal be possible? 3. Did we have to deprecate some methods anyway? Ismael On Tue, Sep 21, 2021 at 10:07 AM Tom Bentley <tbent...@redhat.com> wrote: > Hi Ismael, > > I agree that that is a laudable aim, but I couldn't see a good way of > achieving that while simultaneously allowing us the ability to evolve the > constructor signatures without breaking (or at least having to reason about > the compatibility impact of) test code which instantiates them. Hence the > need to decide whether this is worth addressing, and if so, how. > > In the KIP I prioritised the ability to evolve the API because I think that > was the original intent behind package-private, and I think that's a valid > reason to hide them. If it weren't possible to mock them at all I would > weigh the trade-off differently. > > I wouldn't argue if there was a consensus for making the constructors > public. The overall point would be having consistency. Speaking personally, > I think the constructor issue in KAFKA-13276 snuck past me because I didn't > notice the public modifier having internalised "all those constructors are > package-private". > > Did you have any bright ideas for how to provide more ergonomic mocking > without exposing the constructors? Or do you just value the mocking above > the evolvability in this case? > > Kind regards, > > Tom > > On Tue, Sep 21, 2021 at 1:21 PM Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Tom, > > > > You say: > > > > "While the creation of Admin mocks with package constructors is not > > _ergonomic_, it is _possible_. The example code in KIP-692 requires two > > line of codes for each result instance." > > > > Should we not be aiming to make it ergonomic? > > > > Ismael > > > > On Thu, Sep 9, 2021 at 7:25 AM Tom Bentley <tbent...@redhat.com> wrote: > > > > > Hi, > > > > > > I've written a small KIP-774 that proposes to deprecate public access > to > > > the Admin client's *Result constructors: > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-774%3A+Deprecate+public+access+to+Admin+client%27s+*Result+constructors > > > > > > I'd be grateful for any comments you may have. > > > > > > Kind regards, > > > > > > Tom > > > > > >
