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

take options by reference wherever possible #99

Open
nilslice opened this issue Aug 19, 2023 · 2 comments
Open

take options by reference wherever possible #99

nilslice opened this issue Aug 19, 2023 · 2 comments
Assignees

Comments

@nilslice
Copy link
Member

In the updated adapter.start calls or wherever we're passing in options, we should take the value by reference vs. expecting to own it. particularly in Rust, currently each trace context must make its own new Options, or if cloneable, copy it from elsewhere.

e.g.

- let trace_ctx = adapter.start(_, _, options);
+ let trace_ctx = adapter.start(_, _, &options);

It could also be done in Go so the value isn't copied around, but that's more of a performance/resource optimization than a pesky DX as it is in Rust.

@nilslice
Copy link
Member Author

cc/ @wikiwong, just tagging you on this so we can talk it through later!

@wikiwong
Copy link
Contributor

thanks for capturing this @nilslice ! I went down this direction initially in Rust then ended up with some whacky looking lifetime declarations, could def use some help figuring out how to do this in the me most idiomatic way

As for Go, we are passing Options by reference into NewTraceCtx so we should be good there!

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

No branches or pull requests

2 participants