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


Reply via email to