Admaing commented on PR #29239:
URL: https://github.com/apache/flink/pull/29239#issuecomment-5848426597

   > Thanks for updating the PR!
   > 
   > Mostly LGTM.
   > 
   > Would you mind splitting the change into several commits?
   > 
   > E.g.
   > 
   > ```
   > 1. [FLINK-40504][core] Make PartialWatermark idleness tri-state: an 
unknown/active/idle enum plus hasActiveOutput(), with no behaviour change, 
since "unknown" combines the same way idle = false does today. 
   > This keeps the refactor apart from the behaviour changes.
   > 
   > 2. [FLINK-40504][runtime] Mark WatermarkToDataOutput active on 
non-advancing watermarks: the change to WatermarkToDataOutput and its test.
   > This is the contract fix that needs a release note.
   > 
   > 3. [FLINK-40504][core] Report activity from WatermarksWithIdleness on 
events: the onEvent() change and testMarksActiveOnFirstEventAfterIdleness.
   > It's a @Public class, so a separate commit makes its release note easy to 
trace.
   > 
   > 4. [FLINK-40504][core] Propagate active combined status from 
WatermarkOutputMultiplexer: 
   > the hasActiveOutput() call in updateCombinedWatermark(), 
   > the 4 multiplexer tests, 
   > the new ProgressiveTimestampsAndWatermarksSubtaskIdlenessTest and 
   > the new SourceOperatorEventTimeTest case.
   > ```
   
   Done — split into the four commits you suggested.


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