Thanks for your comments! > - showing the signatures of methods is sufficient (no need to have > implementation details in the KIP). > > - `KafkaAdminClient#close(long duration, TimeUnit unit)` should be > added as deprecated in the KIP, too > > - `AdminClient#close(long duration, TimeUnit unit)` is `abstract` and > the KIP should contain this. I have addressed all comments. Please take a look.
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89070496 -- Chia-Ping On 2018/09/08 19:52:38, "Matthias J. Sax" <matth...@confluent.io> wrote: > Thanks for updating the KIP! > > Some more nits: > > - showing the signatures of methods is sufficient (no need to have > implementation details in the KIP). > > - `KafkaAdminClient#close(long duration, TimeUnit unit)` should be > added as deprecated in the KIP, too > > - `AdminClient#close(long duration, TimeUnit unit)` is `abstract` and > the KIP should contain this. > > > > -Matthias > > > > On 9/8/18 11:22 AM, Chia-Ping Tsai wrote: > >> It's a little hard to read -- it's easier if you just list the methods > >> (without JavaDocs) and indicate if the get deprecated or added. Please > >> don't show a diff as in a patch :) > >> > >> Is there already a JIRA for this? If not, please create on and link it > >> in the KIP. > >> > >> Besides this, I think you can start a VOTE. > > > > Thanks for your suggestions! I have updated KIP-367. > > > > On 2018/09/07 04:27:26, "Matthias J. Sax" <matth...@confluent.io> wrote: > >> Thanks for the KIP. > >> > >> It's a little hard to read -- it's easier if you just list the methods > >> (without JavaDocs) and indicate if the get deprecated or added. Please > >> don't show a diff as in a patch :) > >> > >> Is there already a JIRA for this? If not, please create on and link it > >> in the KIP. > >> > >> Besides this, I think you can start a VOTE. > >> > >> > >> > >> -Matthias > >> > >> On 9/3/18 11:28 PM, Chia-Ping Tsai wrote: > >>> hi Jason > >>> > >>>> Thanks for the KIP. Makes sense to me. Should we make a similar change to > >>>> AdminClient? > >>> > >>> I have updated KIP-367 to address your comment. Could you please take a > >>> look? > >>> > >>> On 2018/08/28 20:13:19, Jason Gustafson <ja...@confluent.io> wrote: > >>>> Thanks for the KIP. Makes sense to me. Should we make a similar change to > >>>> AdminClient? > >>>> > >>>> -Jason > >>>> > >>>> On Tue, Aug 28, 2018 at 2:32 AM, Chia-Ping Tsai <chia7...@apache.org> > >>>> wrote: > >>>> > >>>>> (re-start the thread for KIP-367 because I enter the incorrect topic in > >>>>> first post) > >>>>> > >>>>> hi all > >>>>> > >>>>> I would like to start a discussion of KIP-367 [1]. It is similar to > >>>>> KIP-358 and KIP-266 which is trying to substitute Duration for (long, > >>>>> TimeUnit). > >>>>> > >>>>> [1] https://cwiki.apache.org/confluence/pages/viewpage. > >>>>> action?pageId=89070496 > >>>>> > >>>>> -- > >>>>> Chia-Ping > >>>>> > >>>> > >> > >> > >
