dawidwys commented on a change in pull request #13135: URL: https://github.com/apache/flink/pull/13135#discussion_r475409274
########## File path: docs/dev/stream/operators/index.md ########## @@ -104,8 +104,8 @@ dataStream.filter(new FilterFunction<Integer>() { <p> This transformation returns a <em>KeyedStream</em>, which is, among other things, required to use <a href="{{ site.baseurl }}/dev/stream/state/state.html#keyed-state">keyed state</a>. </p> {% highlight java %} -dataStream.keyBy("someKey") // Key by field "someKey" -dataStream.keyBy(0) // Key by the first element of a Tuple +dataStream.keyBy(value -> value.someKey) // Key by field "someKey", or maybe use getSomeKey Review comment: Let's just use the getter instead of putting it in the comment. ```suggestion dataStream.keyBy(value -> value.getSomeKey()) // Key by field "someKey" ``` ########## File path: docs/dev/types_serialization.md ########## @@ -70,7 +70,7 @@ wordCounts.map(new MapFunction<Tuple2<String, Integer>, Integer>() { } }); -wordCounts.keyBy(0); // also valid .keyBy("f0") +wordCounts.keyBy(value -> value.f0); // .keyBy(0) is @Deprecated Review comment: In my opinion, there is no point in mentioning something that is deprecated. Let's just drop the comment. ########## File path: docs/dev/types_serialization.md ########## @@ -86,11 +86,11 @@ val input = env.fromElements( WordCount("hello", 1), WordCount("world", 2)) // Case Class Data Set -input.keyBy("word")// key by field expression "word" +input.keyBy(_.word) // .keyBy("word") is @Deprecated Review comment: In my opinion, there is no point in mentioning something that is deprecated. Let's just drop the comment. ########## File path: docs/dev/types_serialization.md ########## @@ -238,7 +238,7 @@ Flink tries to infer a lot of information about the data types that are exchange Think about it like a database that infers the schema of tables. In most cases, Flink infers all necessary information seamlessly by itself. Having the type information allows Flink to do some cool things: -* Using POJOs types and grouping / joining / aggregating them by referring to field names (like `dataSet.keyBy("username")`). +* Using POJOs types and grouping / joining / aggregating them by referring to field names (like `dataSet.keyBy(value -> value.username)`). Review comment: We are no longer referring to field names here. Honestly I am not sure if the point makes sense with the `KeySelector` only. The way I read this point is tells you can use field names, which we discourage nowadays. I'd rather remove the point whatsoever. ########## File path: docs/dev/types_serialization.md ########## @@ -86,11 +86,11 @@ val input = env.fromElements( WordCount("hello", 1), WordCount("world", 2)) // Case Class Data Set -input.keyBy("word")// key by field expression "word" +input.keyBy(_.word) // .keyBy("word") is @Deprecated val input2 = env.fromElements(("hello", 1), ("world", 2)) // Tuple2 Data Set -input2.keyBy(0, 1) // key by field positions 0 and 1 +input2.keyBy(value => (value._1, value._2)) // key by field positions 0 and 1 Review comment: We don't key by positions here. Let's remove the comment. ########## File path: docs/dev/types_serialization.md ########## @@ -137,7 +137,7 @@ DataStream<WordWithCount> wordCounts = env.fromElements( new WordWithCount("hello", 1), new WordWithCount("world", 2)); -wordCounts.keyBy("word"); // key by field expression "word" +wordCounts.keyBy(value -> value.word); // .keyBy("word") is @Deprecated Review comment: In my opinion, there is no point in mentioning something that is deprecated. Let's just drop the comment. ########## File path: docs/dev/types_serialization.md ########## @@ -153,7 +153,7 @@ val input = env.fromElements( new WordWithCount("hello", 1), new WordWithCount("world", 2)) // Case Class Data Set -input.keyBy("word")// key by field expression "word" +input.keyBy(_.word) // .keyBy("word") is @Deprecated Review comment: In my opinion, there is no point in mentioning something that is deprecated. Let's just drop the comment. ########## File path: docs/learn-flink/etl.md ########## @@ -149,6 +149,7 @@ this would mean doing some sort of GROUP BY with the `startCell`, while in Flink {% highlight java %} rides .flatMap(new NYCEnrichment()) + // .keyBy("fieldname") is @Deprecated, could use .keyBy(value -> value.startCell) Review comment: Why don't we just use the correct version instead? Again I can't see a reason why we should even mention deprecated methods. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org