Skip to content
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

Avoid duplicating With[Start|Shutdown] in processorhelper #2788

Closed
bogdandrutu opened this issue Mar 24, 2021 · 6 comments
Closed

Avoid duplicating With[Start|Shutdown] in processorhelper #2788

bogdandrutu opened this issue Mar 24, 2021 · 6 comments
Assignees
Labels
discussion-needed Community discussion needed

Comments

@bogdandrutu
Copy link
Member

Rather than needing to duplicate every componenthelper.Option here, might it be better to have something like this:

func WithComponentOptions(opts componenthelper.Option...) Option {
  return func( o *baseSettings) {
    o.componentOptions = opts
  }
}
@bogdandrutu
Copy link
Member Author

See #2778 (comment)

punya added a commit to punya/opentelemetry-collector that referenced this issue Mar 26, 2021
punya added a commit to punya/opentelemetry-collector that referenced this issue Mar 26, 2021
punya added a commit to punya/opentelemetry-collector that referenced this issue Mar 26, 2021
@alolita
Copy link
Member

alolita commented May 12, 2021

@bogdandrutu can we close this issue given @punya's comments?

@alolita alolita added the discussion-needed Community discussion needed label May 18, 2021
@alolita
Copy link
Member

alolita commented Jun 24, 2021

@bogdandrutu should this issue be closed?

@alolita
Copy link
Member

alolita commented Jun 25, 2021

This seems like a style-based decision. @Aneurysm9 had made a suggestion so @punya can we chat w Anthony and figure out the best practice in this case :-) ty!

@Aneurysm9
Copy link
Member

I think @punya covered the pros/cons well in this comment. The important question is probably whether/how often we expect to add new options that can benefit from this change. If we don't, then maybe it's not worth it. The value of the change goes up with each additional option that can utilize it. v0v

@punya
Copy link
Member

punya commented Jun 25, 2021

We haven't added any more "generic" options like Start and Shutdown since the time this issue first came up. To me, that's decent evidence that we don't need to optimize for ease of adding future options of this sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion-needed Community discussion needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants