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 >
