Wrap component opts for exporter and processor #2825
Conversation
punya
force-pushed
the
wrap-component-opts
branch
from
a84536e to
fd0a682
Compare
punya
force-pushed
the
wrap-component-opts
branch
from
fd0a682 to
b350d8d
Compare
| func WithShutdown(shutdown componenthelper.Shutdown) Option { | ||
| return func(o *baseSettings) { | ||
| o.componentOptions = append(o.componentOptions, componenthelper.WithShutdown(shutdown)) | ||
| o.componentOptions = append(o.componentOptions, opts...) |
There was a problem hiding this comment.
Do we want to call append here or overwrite?
There was a problem hiding this comment.
I chose append on purpose, but I don't have a strong preference.
There was a problem hiding this comment.
If a piece of code configures startup and shutdown options far apart for any reason, the append approach gives them a way forward (because they can replace WithStart -> WithComponentOptions(c.WithStart) etc.) rather than forcing them to pull the two calls close together. As I said, it's not a big deal either way.
|
Is this change actually worth making? Pros:
Cons:
I suggest that we decline this PR and WONTFIX the two issues. |
Codecov Report
@@ Coverage Diff @@
## main #2825 +/- ##
==========================================
- Coverage 91.77% 91.77% -0.01%
==========================================
Files 281 281
Lines 15131 15129 -2
==========================================
- Hits 13886 13884 -2
Misses 852 852
Partials 393 393
Continue to review full report at Codecov.
|
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closing because a) this doesn't seem high priority and b) we couldn't come to a conclusion on which API is better. |
Fixes #2787
Fixes #2788