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
