-
Notifications
You must be signed in to change notification settings - Fork 43
Seal public traits #1037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Seal public traits #1037
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM barring the PassThroughPropagator
thing
core/common/src/main/scala/org/typelevel/otel4s/context/propagation/PassThroughPropagator.scala
Show resolved
Hide resolved
val callback = cb { (value, attributes) => | ||
observable.record(cast(value), attributes) | ||
val measurement = new ObservableMeasurement.Unsealed[F, A] { | ||
def record(value: A, attributes: Attributes): F[Unit] = | ||
observable.record(cast(value), attributes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's an interesting side effect of this change
dfd01b5
to
3a41e63
Compare
A few items remain 'unsealed':
wrt
However, I'm unsure if it's worth the hassle, as it will affect third-party integration, which is a major inconvenience. |
b16a800
to
d42df76
Compare
I don't think we should seal the |
Closes #1029