Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial implementation of the logging SDK spec #788
Initial implementation of the logging SDK spec #788
Changes from 47 commits
7b6ed21
56edfb5
54a41da
6d0875c
1a2b532
c7631dd
8c23c09
085b598
5c8539e
1b9f870
4bb4216
4ebe135
d0016f1
10c69c2
2f0ae2c
ecf13cf
5c1b0b7
092376f
3eb34cd
0e0bbbf
22f5ab5
d3b7b15
aeb53ff
0593aa5
fec1cdf
e0fae82
4777062
a8e64d6
fbe8c4e
d85295d
b7dbee5
93bca4f
3a8f49a
e20e697
e77388a
0d88043
3a34dea
5f75512
1656a91
a9b41aa
26b2b1e
283f932
322da85
93f5acd
070b496
173fc3d
b2fc857
47731ca
a617fd5
ad676ac
5efc75d
59469cb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This can be:
name: impl Into<Cow<'static, str>>,
this will allow us to directly pass string literal as
name
argument.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.
The
name
argument can be:name: impl Into<Cow<'static, str>>
this will allow us to directly pass string literal as argument.
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.
More a comment for when we look to stabilize, but non-exhaustive is a double edged sword because while adding a new enum value is considered breaking without it because code will fail to compile. It might be worse to have people think they covered all errors and then find out at runtime.
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.
Agree, better to fail early at compile time. Probably we can remove non-exhaustive attribute before releasing stable version of logs.
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 think this should be in the SDK only. Its probably a mistake that was already in traces api. See #1042