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

(RFC) Rename NewConfig in sdk/resource to better fit its function #1263

Closed
MrAlias opened this issue Oct 17, 2020 · 5 comments
Closed

(RFC) Rename NewConfig in sdk/resource to better fit its function #1263

MrAlias opened this issue Oct 17, 2020 · 5 comments
Labels
area:resources Part of OpenTelemetry resources pkg:API Related to an API package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Oct 17, 2020

Currently NewConifg doesn't return a config and there are likely better names for this function. I'm guessing a better name would reflect the Resource being returned and the use of Detectors to create it. Possibly NewFromDetectors to compliment New.

We should brainstorm names here, form consensus, and then implement a new name.

Originally posted by @MrAlias in #1235 (comment)

@jmacd
Copy link
Contributor

jmacd commented Oct 22, 2020

I'd like to fix this in the open PR. I've proposed we replace New() with this new method in #1235. How does this sound?

@jmacd
Copy link
Contributor

jmacd commented Oct 22, 2020

Maybe we could name a new helper with signature NewFromAttributes(...label.KeyValue) *Resource which calls the more-general New()?

@MrAlias
Copy link
Contributor Author

MrAlias commented Oct 23, 2020

I'd like to fix this in the open PR. I've proposed we replace New() with this new method in #1235. How does this sound?

Maybe we could name a new helper with signature NewFromAttributes(...label.KeyValue) *Resource which calls the more-general New()?

I am in support of both of these. 👍

@Aneurysm9
Copy link
Member

I'd like to fix this in the open PR. I've proposed we replace New() with this new method in #1235. How does this sound?

Maybe we could name a new helper with signature NewFromAttributes(...label.KeyValue) *Resource which calls the more-general New()?

I am in support of both of these. 👍

I second that support.

@MrAlias MrAlias added area:resources Part of OpenTelemetry resources pkg:API Related to an API package priority:p1 labels Oct 29, 2020
@MrAlias MrAlias added this to To do in P1 Burndown via automation Oct 29, 2020
@MrAlias MrAlias added this to To do in OpenTelemetry Go RC via automation Oct 29, 2020
@MrAlias MrAlias added this to the RC1 milestone Oct 29, 2020
@MrAlias
Copy link
Contributor Author

MrAlias commented Nov 5, 2020

Fixed in #1235

@MrAlias MrAlias closed this as completed Nov 5, 2020
P1 Burndown automation moved this from To do to Done Nov 5, 2020
OpenTelemetry Go RC automation moved this from To do to Done Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:resources Part of OpenTelemetry resources pkg:API Related to an API package
Projects
No open projects
Development

No branches or pull requests

3 participants