-
Notifications
You must be signed in to change notification settings - Fork 772
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
[sdk-logs] Update LogRecord for Logger API additions #4456
[sdk-logs] Update LogRecord for Logger API additions #4456
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4456 +/- ##
==========================================
- Coverage 83.41% 83.35% -0.06%
==========================================
Files 313 313
Lines 12485 12512 +27
==========================================
+ Hits 10414 10430 +16
- Misses 2071 2082 +11
|
/// <summary> | ||
/// Gets or sets the log <see cref="LogRecordSeverity"/>. | ||
/// </summary> | ||
internal LogRecordSeverity? Severity |
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.
Nullable or just Unspecified?
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 feel like Nullable
is more the .NET way for saying a value type is unspecified but originally I didn't have LogRecordSeverity.Unspecified
defined 🤣 If you think it would be better to just default to that and drop Nullable I'm good with it.
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'm ok leaving this for now. Good question though.. I think we should opt for one or the other: nullable or Unspecified
. I think I'll open an issue where we can capture a punch list of small questions like this for us to circle back to before we declare stable.
Relates to #4433
Changes
LogRecord
for additions ofLogger
,SeverityText
, andSeverity
.Merge requirement checklist