Github user bowenli86 commented on the issue:
https://github.com/apache/flink/pull/5069
My apologies! Silly mistake at late night when the brain is quite
inactive I forgot to check the files history, and thought this PR (which
shows as latest commit) added those examples...
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5069
@bowenli86 do you mean the `ProduceIntoKinesis` and `ConsumeFromKinesis`
classes?
I agree that we should move them to `/test`, but we should consider that as
a separate issue than this one.
Github user bowenli86 commented on the issue:
https://github.com/apache/flink/pull/5069
@tzulitai @yew1eb I just saw the code in src tree and traced back to this
PR. I feel it's really bad to have runnable examples (with java main method) in
a lib jar that Flink distributes.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5069
Ah, I just realized that my comment on the last review is irrelevant, since
`org.apache.flink.streaming.util.serialization.SerializationSchema` extends
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5069
LGTM, merging this ..
---
Github user yew1eb commented on the issue:
https://github.com/apache/flink/pull/5069
@tzulitai, thanks for your review. I have updated the PR according to your
comments.
---
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5069
Thanks for the PR @yew1eb! I have one comment regarding API-breaking nature
of the change.
---
