Hi Chia-Ping, Thanks for the KIP. I have just a few minor comments:
1. The Javadoc for the new interface says "an `InputStream` received via `init`" but the interface doesn't have an init() method. I guess you meant configure()? 2. The Javadoc should mention the need for implementations to have a public nullary constructor. 3. On the matter of configure(): While it doesn't add any functionality, it would be more consistent with other plugins if this interface extended Configurable, and the InputStream was then passed via some other method (`readFrom(InputStream)` perhaps). If nothing else it would make it harder to overlook this interface when making changes which apply to all plugins. To be honest, I'm not entirely convinced myself, but I thought we should at least consider it and add it to the rejected alternatives if we decide against it. Thanks again, Tom On Thu, 16 Feb 2023 at 11:36, Federico Valeri <fedeval...@gmail.com> wrote: > Hi Chia-Ping, thanks for updating the KIP. > > I would only add that we plan to remove the old trait in the next > major release. I think it's better to be explicit about this. > > On Thu, Feb 16, 2023 at 11:34 AM Chia-Ping Tsai <chia7...@apache.org> > wrote: > > > > Dear all, > > > > I have updated the KIP to address comments. PTAL > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=158866569 > >
