-
Notifications
You must be signed in to change notification settings - Fork 73
Begin altering initialize to use a type-safe DSL #1292
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
==========================================
+ Coverage 63.87% 63.97% +0.09%
==========================================
Files 150 154 +4
Lines 3070 3103 +33
Branches 305 304 -1
==========================================
+ Hits 1961 1985 +24
- Misses 1028 1038 +10
+ Partials 81 80 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b82936c
to
538097b
Compare
538097b
to
d99d6a6
Compare
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.
Looks great, cheers!
globalAttributes: (() -> Attributes)? = null, | ||
diskBuffering: (DiskBufferingConfigurationSpec.() -> Unit)? = null, |
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.
Maybe a discussion for another PR, though I'm curious about the decision process to decide which parameters are left on the initializer method's arguments, vs which ones are placed into the Configuration
dsl?
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.
@LikeTheSalad I'd like to move basically every parameter apart from Application
onto the Configuration
DSL. I wanted to keep the PR size small particularly as other folks had PRs in flight, and should be able to open another PR towards the end of the week that moves the rest.
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.
Sounds good to me, thanks for the clarification.
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.
I've moved the rest over in #1313
Goal
Builds on #1291 by continuing to convert
initialize
to use a type-safe DSL. The last commit contains the changes for this PR specifically - the first commit contains code that can be reviewed separately in #1291.I've converted everything apart from
OtelRumConfig
to use a type-safe DSL calledOpenTelemetryConfiguration
.OtelRumConfig
will be converted in a future PR that I've yet to complete. An example of how this looks in practice can be found below: