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

Fixes Problem Details casing bug (#59396) #59876

Merged
merged 2 commits into from
Jan 15, 2025
Merged

Conversation

scottlwalker
Copy link
Contributor

Problem Details casing bug (#59396)

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Fixes bug #59396

Made recommended changes and created unit tests to verify that changes match intended result for traceId.

Fixes #{bug number} (in this specific format)

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jan 14, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 14, 2025
@scottlwalker
Copy link
Contributor Author

@dotnet-policy-service agree

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for making this contribution! 🙇🏽

@captainsafia captainsafia requested a review from Copilot January 15, 2025 00:28

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs:59

  • The variable name 'tradeIdKeyName' appears to be a typo. It should be renamed to 'traceIdKeyName'.
var tradeIdKeyName = _serializerOptions.PropertyNamingPolicy?.ConvertName("traceId") ?? "traceId";
@captainsafia captainsafia enabled auto-merge (squash) January 15, 2025 00:30
@captainsafia captainsafia merged commit 8aec8fd into dotnet:main Jan 15, 2025
26 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Jan 15, 2025
@scottlwalker
Copy link
Contributor Author

LGTM! Thanks for making this contribution! 🙇🏽

Sure no problem it was fun!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants