+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

Reply via email to