+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 > > >
