[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-12-07 Thread bowenli86
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...

Opened FLINK-8218. Sincere apologies again!


---


[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-12-06 Thread tzulitai
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. Could you open a JIRA for that?


---


[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-12-06 Thread bowenli86
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.

@yew1eb can you move the examples to `/test`? I believe that's the right 
way of adding examples


---


[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-11-30 Thread tzulitai
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 
`org.apache.flink.api.common.serialization.SerializationSchema`, meaning that 
there is no API-breaking change.

Sorry about this. The PR is good to go without the extra follow-up fix. 
Merging the first commit.


---


[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-11-29 Thread tzulitai
Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5069
  
LGTM, merging this ..


---


[GitHub] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-11-29 Thread yew1eb
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] flink issue #5069: [FLINK-8149][kinesis] Replace usages of deprecated Serial...

2017-11-27 Thread tzulitai
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.


---