You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2017/07/03 17:53:51 UTC

[GitHub] flink issue #4241: [FLINK-7015] [streaming] separate OperatorConfig from Str...

Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/4241
  
    In general, it is a good idea to clean that up a bit. That code needed some refactoring.
    
    As @aljoscha pointed out, that is an intricate part, so we need to review that carefully. To be able to review that, we need a good description of what the refactoring did. Trying to understand that from the code alone will take a lot of time and most likely slow down the merging of this by a lot.
    
    I would suggest to write
      - what exactly was addressed
      - what is the new design
      - does it change behavior, or is it only code abstraction internally
    
    
    As another point: I just took a quick look at the `OperatorConfig`. I think I would be nice to move away from the model of having a Map of "configKey" to values, but just to make this a serializable class with the respective fields. That would be simpler to understand and maintain.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---