LGTM. +1 (binding) -- Harsha
On Oct 18, 2018, 5:53 PM -0700, Matthias J. Sax <matth...@confluent.io>, wrote: > +1 (binding) > > Thanks for the KIP. And thanks for bumping the thread regularly. As > 2.1.0 and 2.0.1 releases are running atm, it takes some time to get > attention. > > -Matthias > > On 10/18/18 11:47 AM, Yishun Guan wrote: > > Bumping this thread up again, thanks! > > On Tue, Oct 16, 2018 at 11:24 AM Yishun Guan <gyis...@gmail.com> wrote: > > > > > > Bumping this thread up again, thanks! > > > > > > On Fri, Oct 12, 2018, 4:53 PM Colin McCabe <cmcc...@apache.org> wrote: > > > > > > > > On Fri, Oct 12, 2018, at 15:45, Yishun Guan wrote: > > > > > Hi Colin, > > > > > > > > > > Thanks for your suggestions. I have modified the current KIP with your > > > > > comments. However, I still think I should keep the entire list, > > > > > because it > > > > > is a good way to keep track of which class need to be change, and > > > > > others > > > > > can discuss if changes on these internal classes are necessary? > > > > > > > > Hi Yishun, > > > > > > > > I guess I don't feel that strongly about it. If you want to keep the > > > > internal classes in the list, that's fine. They don't really need to be > > > > in the KIP but it's OK if they're there. > > > > > > > > Thanks for working on this. +1 (binding). > > > > > > > > best, > > > > Colin > > > > > > > > > > > > > > Thanks, > > > > > Yishun > > > > > > > > > > On Fri, Oct 12, 2018 at 11:42 AM Colin McCabe <cmcc...@apache.org> > > > > > wrote: > > > > > > > > > > > Hi Yishun, > > > > > > > > > > > > Thanks for looking at this. > > > > > > > > > > > > Under "proposed changes," it's not necessary to add a section where > > > > > > you > > > > > > demonstrate adding "implements AutoCloseable" to the code. We know > > > > > > what > > > > > > adding that would look like. > > > > > > > > > > > > Can you create a full, single, list of all the classes that would be > > > > > > affected? It's not necessary to write who suggested which classes > > > > > > in the > > > > > > KIP. Also, I noticed some of the classes here are in "internals" > > > > > > packages. Given that these are internal classes that aren't part of > > > > > > our > > > > > > API, it's not necessary to add them to the KIP, I think. Since they > > > > > > are > > > > > > implementation details, they can be changed at any time without a > > > > > > KIP. > > > > > > > > > > > > The "compatibility" section should have a discussion of the fact > > > > > > that we > > > > > > can add the new interface without requiring any > > > > > > backwards-incompatible > > > > > > changes at the source or binary level. In particular, it would be > > > > > > good to > > > > > > highlight that we are not renaming or changing the existing "close" > > > > > > methods. > > > > > > > > > > > > Under "rejected alternatives" we could explain why we chose to > > > > > > implement > > > > > > AutoCloseable rather than Closeable. > > > > > > > > > > > > cheers, > > > > > > Colin > > > > > > > > > > > > > > > > > > On Thu, Oct 11, 2018, at 13:48, Yishun Guan wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Just to bump this voting thread up again. Thanks! > > > > > > > > > > > > > > Best, > > > > > > > Yishun > > > > > > > On Fri, Oct 5, 2018 at 12:58 PM Yishun Guan <gyis...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > I think we have discussed this well enough to put this into a > > > > > > > > vote. > > > > > > > > > > > > > > > > Suggestions are welcome! > > > > > > > > > > > > > > > > Best, > > > > > > > > Yishun > > > > > > > > > > > > > > > > On Wed, Oct 3, 2018, 2:30 PM Yishun Guan <gyis...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi All, > > > > > > > > > > > > > > > > > > I want to start a voting on this KIP: > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308 > > > > > > > > > > > > > > > > > > Here is the discussion thread: > > > > > > > > > > > > > > > https://lists.apache.org/thread.html/9f6394c28d3d11a67600d5d7001e8aaa318f1ad497b50645654bbe3f@%3Cdev.kafka.apache.org%3E > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Yishun > > > > > > >
