[ 
https://issues.apache.org/jira/browse/BEAM-11105?focusedWorklogId=764427&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-764427
 ]

ASF GitHub Bot logged work on BEAM-11105:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 29/Apr/22 19:19
            Start Date: 29/Apr/22 19:19
    Worklog Time Spent: 10m 
      Work Description: lostluck commented on code in PR #17476:
URL: https://github.com/apache/beam/pull/17476#discussion_r862082099


##########
sdks/go/pkg/beam/core/sdf/sdf.go:
##########
@@ -96,3 +98,12 @@ type WatermarkEstimator interface {
        // splits or checkpoints to advance the output watermark of the 
restriction's stage.
        CurrentWatermark() time.Time
 }
+
+// ToWatermarkEstimator is an interface used to represent a user defined 
watermark estimator that
+// has the ability to observe timestamps of elements outputted from a ParDo's 
emit function.
+type ToWatermarkEstimator interface {

Review Comment:
   If TO is supposed to be an acronym for TimestampObserving, please capitalize 
it accordingly.  
   I'd also recommend simplifying it to `TimestampObserver`. Yes it only 
applies to WatermarkEstimators with the embed, but that can be clarified in the 
type documentation, rather than the whole name.



##########
sdks/go/pkg/beam/core/sdf/sdf.go:
##########
@@ -96,3 +98,12 @@ type WatermarkEstimator interface {
        // splits or checkpoints to advance the output watermark of the 
restriction's stage.
        CurrentWatermark() time.Time
 }
+
+// ToWatermarkEstimator is an interface used to represent a user defined 
watermark estimator that
+// has the ability to observe timestamps of elements outputted from a ParDo's 
emit function.
+type ToWatermarkEstimator interface {

Review Comment:
   If TO is supposed to be an acronym for TimestampObserving, please capitalize 
it accordingly.  As it stands, it sounds like a type that converts types to 
WatermarkEstimators.
   
   I recommend simplifying it to `TimestampObserver`. Yes ,it only applies to 
WatermarkEstimators with the embed, but that can also be clarified in the type 
documentation, rather than the name.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 764427)
    Time Spent: 9h 50m  (was: 9h 40m)

> [Go SDK] Watermark Estimation
> -----------------------------
>
>                 Key: BEAM-11105
>                 URL: https://issues.apache.org/jira/browse/BEAM-11105
>             Project: Beam
>          Issue Type: Sub-task
>          Components: sdk-go
>            Reporter: Robert Burke
>            Assignee: Danny McCormick
>            Priority: P3
>          Time Spent: 9h 50m
>  Remaining Estimate: 0h
>
> Allow self checkpointing SplittableDoFns to specify a watermark estimator.
> (To be updated once [https://github.com/apache/beam/pull/13160] is merged and 
> the programming guide updated with SDF content.)



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to