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

Update IHttpClientLogger.cs and IHttpClientAsyncLogger.cs remarks #109618

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

danespinosa
Copy link
Contributor

@danespinosa danespinosa commented Nov 7, 2024

Updated IHttpClientLogger remarks to provide clearer documentation to a developer implementing this interface
This PR pairs up with the doc api pr dotnet/dotnet-api-docs#10653

Updated IHttpClientLogger remarks to provide clearer documentation to a developer implementing this interface
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 7, 2024
Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

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

LGTM, but I believe the actual API docs still need to be updated manually in https://github.com/dotnet/dotnet-api-docs

@danespinosa
Copy link
Contributor Author

@CarnaViire would you have the direct link to the api doc? I searched and searched without any results, that's how I ended up fixing it here 😂

@danespinosa
Copy link
Contributor Author

@CarnaViire would you have the direct link to the api doc? I searched and searched without any results, that's how I ended up fixing it here 😂

@CarnaViire I was looking wrong, I have found the doc here, I'll send the PR there too https://github.com/dotnet/dotnet-api-docs/blob/c4ea6458ca24b6cf5c17292db5136a974fb3a5f7/xml/Microsoft.Extensions.Http.Logging/IHttpClientLogger.xml#L4

Updated remarks to provide the developer implementing this interface clearer documentation
@danespinosa danespinosa changed the title Update IHttpClientLogger.cs remarks Update IHttpClientLogger.cs and IHttpClientLoggerAsync.c remarks Nov 7, 2024
@danespinosa danespinosa changed the title Update IHttpClientLogger.cs and IHttpClientLoggerAsync.c remarks Update IHttpClientLogger.cs and IHttpClientLoggerAsync.cs remarks Nov 7, 2024
@danespinosa danespinosa changed the title Update IHttpClientLogger.cs and IHttpClientLoggerAsync.cs remarks Update IHttpClientLogger.cs and IHttpClientAsyncLogger.cs remarks Nov 7, 2024
@danespinosa
Copy link
Contributor Author

danespinosa commented Nov 7, 2024

LGTM, but I believe the actual API docs still need to be updated manually in https://github.com/dotnet/dotnet-api-docs

Ok, I also fixed IHttpAsyncClientLogger.cs too and added the doc pr here dotnet/dotnet-api-docs#10653 but it seems here is the right place?

@CarnaViire
Copy link
Member

CarnaViire commented Nov 8, 2024

Ok, I also fixed IHttpAsyncClientLogger.cs too

Thanks!

and added the doc pr here dotnet/dotnet-api-docs#10653 but it seems here is the right place?

Hmm.... Sorry for that. The process was changing back and forth over the last couple of years; I was under impression that the changes in the triple-slash docs don't sync automatically (only manually -- usually via bulk PRs), but maybe that was an outdated info. Anyway, let's go forward with that one and see if it syncs.

@danespinosa
Copy link
Contributor Author

Ok, I also fixed IHttpAsyncClientLogger.cs too

Thanks!

and added the doc pr here dotnet/dotnet-api-docs#10653 but it seems here is the right place?

Hmm.... Sorry for that. The process was changing back and forth over the last couple of years; I was under impression that the changes in the triple-slash docs don't sync automatically (only manually -- usually via bulk PRs), but maybe that was an outdated info. Anyway, let's go forward with that one and see if it syncs.

Thanks @CarnaViire no worries I have updated the dotnet api docs before too so I also got confused 😂

@CarnaViire
Copy link
Member

/ba-g second reported issue is actually the same infra error (Build wasi-wasm linux Release LibraryTests_Smoke timeout) as the already linked dotnet/dnceng#3008

@CarnaViire CarnaViire merged commit 7ee5b7d into dotnet:main Nov 11, 2024
81 of 83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-HttpClientFactory 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