Hello Shawn, Could you close this voting thread with a tally on the votes (you can search for other voting threads as a reference)?
Guozhang On Wed, Dec 19, 2018 at 1:15 AM Matthias J. Sax <matth...@confluent.io> wrote: > Sorry for the delay. Needed to read the KIP and code multiple times to > wrap my head around it. > > Thanks for the KIP. > > > +1 (binding) > > > -Matthias > > > On 12/13/18 1:19 PM, Damian Guy wrote: > > +1 (binding) > > > > On Wed, 12 Dec 2018 at 13:27, Adam Bellemare <adam.bellem...@gmail.com> > > wrote: > > > >> +1 (non-binding) from me. Looks like a pretty clear-cut case. > >> > >> > >> > >> On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen <shavvnnngu...@gmail.com> > >> wrote: > >> > >>> Thanks for the feedback Guozhang! I updated the KIP. > >>> > >>> In the meantime, could I ask for additional binding votes/approval on > >> this > >>> KIP proposal? > >>> > >>> Shawn > >>> > >>> On Thu, Dec 6, 2018 at 1:22 PM Liquan Pei <liquan...@gmail.com> wrote: > >>> > >>>> +1 (non-binding) > >>>> > >>>> On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang <wangg...@gmail.com> > >> wrote: > >>>> > >>>>> Hello Shawn, > >>>>> > >>>>> Thanks for the writeup. I've made a pass over it and here are some > >> minor > >>>>> comments: > >>>>> > >>>>> 1) As we discussed in the PR: > >> https://github.com/apache/kafka/pull/5307 > >>> , > >>>>> the public APIs that we will add is > >>>>> > >>>>> In WindowedSerdes: > >>>>> ``` > >>>>> static public <T> Serde<Windowed<T>> > >>> timeWindowedChangelogSerdeFrom(final > >>>>> Class<T> type, final long windowSize) > >>>>> ``` > >>>>> > >>>>> In TimeWindowedSerde > >>>>> ``` > >>>>> TimeWindowedSerde forChangelog(final boolean); > >>>>> ``` > >>>>> > >>>>> Other classes such as WindowedKeySchema are internal classes for > >>>>> implementation details and hence do not need to be listed in the wiki > >> as > >>>>> public APIs. > >>>>> > >>>>> > >>>>> 2) The wiki doc may reads a bit confusing for audience who are not > >>>>> familiar > >>>>> with the PR, since we mentioned the "forChangelog()" function and the > >>>>> "isChangelog" parameter without clear definitions, but only explained > >>> what > >>>>> it is later in the docs as java code examples. I think rephrasing the > >>>>> early > >>>>> paragraphs to explain a bit more why we will add a new internal field > >>>>> along > >>>>> with a setter, its semantics (its default value and how > >> deserialization > >>>>> will be different depending on that) would be better. > >>>>> > >>>>> Otherwise, I'm +1 on the KIP, thanks! > >>>>> > >>>>> > >>>>> Guozhang > >>>>> > >>>>> > >>>>> On Wed, Dec 5, 2018 at 8:18 AM Shawn Nguyen <shavvnnngu...@gmail.com > > > >>>>> wrote: > >>>>> > >>>>>> Hey all, > >>>>>> > >>>>>> I wanted to start a vote on approval of KIP-393 > >>>>>> < > >>>>>> > >>>>> > >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-393%3A+Time+windowed+serde+to+properly+deserialize+changelog+input+topic > >>>>>>> > >>>>>> to > >>>>>> fix the current time windowed serde for properly deserializing > >>> changelog > >>>>>> input topics. Let me know what you guys think. > >>>>>> > >>>>>> Thanks, > >>>>>> Shawn > >>>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> -- Guozhang > >>>>> > >>>> > >>>> > >>>> -- > >>>> Liquan Pei > >>>> Software Engineer, Confluent Inc > >>>> > >>> > >> > > > > -- -- Guozhang
