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