xintongsong commented on PR #26334:
URL: https://github.com/apache/flink/pull/26334#issuecomment-2745321714

   > According to the design of RpcServiceBuilder, the implementation should 
follow the style of chain programming.
   I'm not sure about this. The builder style design makes `RpcServiceBuilder` 
*CAN* be used in a chaining style programing, but that doesn't mean we *SHOULD* 
follow the chaining style. In fact, one of the advantages of the builder style 
is that you don't need to set all the parameters at once. By strictly following 
the chaining style, we lose that flexibility, and having to changing the 
null-handling logics is a prove of that. So TBH, I don't think this PR makes 
any improvement to the code style.
   
   Moreover, even if this does improve the code style, I don't think it makes 
sense to take the risk of breaking a fundamental component that has been stable 
and not touched for years (no matter how little the risk is), and pay the 
efforts to implement and review the change, just for a code style improvement 
that does not help users with any actual behavior changes.
   
   So overall I'd be -1 for merging this PR.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to