[GitHub] [flink] dawidwys commented on a change in pull request #12201: [hotfix] Remove raw class usages in Configuration.

[GitBox <gi...@apache.org>]

dawidwys commented on a change in pull request #12201:
URL: https://github.com/apache/flink/pull/12201#discussion_r426407452



##########
File path: flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -909,12 +909,10 @@ private void loggingFallback(FallbackKey fallbackKey, ConfigOption<?> configOpti
 			List<String> listOfRawProperties = StructuredOptionsSplitter.splitEscaped(o.toString(), ',');
 			return listOfRawProperties.stream()
 				.map(s -> StructuredOptionsSplitter.splitEscaped(s, ':'))
-				.map(pair -> {
+				.peek(pair -> {

Review comment:
       Peek and map behave exactly the same in respect to the use cases described in that thread. I'd say the thread discusses the declarative aspect of the stream API. 
   
   In a code:
   ```
   stream()
   .map()/peek()
   .count()
   ```
   Neither `map` nor `peek` will be executed (depends on the jvm though) as they cannot change the result of `count()`. The linked thread rather compares `forEach` vs `peek`, in my opinion.
   
   In our use case particularly, the purpose of the `peek` is to add a sanity check right before the collection. In this case in my opinion it is absolutely safe to use `peek` and the code with `peek` is no different than previous version with `map` (minus the return value).
   
   Personally, I'd prefer to change it to `peek` as it removes IDE warning.




----------------------------------------------------------------
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:
users@infra.apache.org