dotnet / dotnet/docs

HttpClientFactory doc improvements

Open
#44,730 0 comments 0 reactions 1 assignee Claimed by @CarnaViire View on GitHub
dotnet-fundamentals/svc dotnet-networking/subsvc
Dominant language
No language data
Stars
4.8k
Forks
6.1k
Avg merge
19h 10m
Merged PRs (30d)
268

Description

### Describe the issue or suggestion

Follow-ups from https://github.com/dotnet/docs/pull/44533:

- [ ] 1. Simplify three main steps example description



https://github.com/dotnet/docs/pull/44533#discussion_r1934371122

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L51

This sentence feels hard to read and interpret. Would it make sense to rewrite the beginning of the paragraph as following?

> Consider the `IHttpClientFactory`-related code from the [Basic Usage](#basic-usage) example. The code snippet illustrates the three main steps using the _Keyed DI approach_:
>
> 1. Registration
> 1. Obtaining the configured `HttpClient` instance
> 1. Using the obtained client instance
>
> ```csharp
> services.AddHttpClient("github", /* ... */).AddAsKeyed(); // (1)
> app.MapGet("/", ([FromKeyedServices("github")] HttpClient httpClient) => // (2)
> //httpClient.Get.... // (3)
> ```

- [ ] 2. Link captive dependency term

https://github.com/dotnet/docs/pull/44533#discussion_r1934397908

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L100

I see that the term captive dependency is defined later in the doc. Would it make sense to link that paragraph?

- [ ] 3. Use emojis in titles

https://github.com/dotnet/docs/pull/44533#discussion_r1934406472

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L138

Not sure if it fits our guidelines but for me it would read better if we used emojis in the titles to raise attention instead of the nested `[!IMPORTANT]` sections.

```suggestion
### ❌ Avoid captive dependency
```

- [ ] 4. Separate code blocks for better parsing

https://github.com/dotnet/docs/pull/44533#discussion_r1934428136

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L189

For me the diff feels harder to parse than if we had two separate code blocks (preferably with detailed explanations in also comments rather then just in the text below).

- [ ] 5. Reduce overuse of highlighting

https://github.com/dotnet/docs/pull/44533#discussion_r1934435543

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L236

I understand that HttpClientFactory is full of traps so all these sections are important, but it feels like highlighting is heavily overused in this paragraph, so it somewhat loses its purpose.

- [ ] 6. Present examples of traps

https://github.com/dotnet/docs/pull/44533#discussion_r1934436168

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L237

Would it make sense to present examples of the traps?

- [ ] 7. Avoid feeling words

https://github.com/dotnet/docs/pull/44533#discussion_r1935271744

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L19

- I'd avoid feeling words like "unsurprisingly" here, and I saw "nasty" somewhere later in the doc.

- What does this has to do with Keyed DI:
> and the supporting infrastructure can also be a tangible overhead in certain scenarios (for example, on mobile platforms).

???

- [ ] 8. Clarify "request handler"

https://github.com/dotnet/docs/pull/44533#discussion_r1935280150

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L38

What does "request handler" mean here? It's confusing because usually the handlers are put inside the client, not the other way around. So either I'm missing something or it's used here to mean something else, which I'm also missing.

- [ ] 9. Explicitly use "scoped"

https://github.com/dotnet/docs/pull/44533#discussion_r1935290140

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L103

I know scoped is default, but I'd use it here explicitly or used a different name. Some people might get confused and think that the string "scoped" has any meaning appart from just being the name.

- [ ] 10. Add small example

https://github.com/dotnet/docs/pull/44533#discussion_r1935297877

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L129

This is not related to the previous example, is it? It took me a while to understand what it's saying, so maybe another small example wouldn't hurt.

- [ ] 11. Define "reasonable" and link to API docs

https://github.com/dotnet/docs/pull/44533#discussion_r1935306641

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L149

What does "reasonable" mean? "Small enough value to observe and react to DNS changes regularly"...
Also, I'd add a link to `PooledConnectionLifetime` API ref docs.

- [ ] 12. Link Singleton and Transient pitfalls

https://github.com/dotnet/docs/pull/44533#discussion_r1935307885

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L163

> Singleton and Transient pitfalls

Link?

- [ ] 13. Remove "as expected"

https://github.com/dotnet/docs/pull/44533#discussion_r1935309031

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L166

"as expected" is unnecessary here.

- [ ] 14. Remove "nasty", "unfortunately"

https://github.com/dotnet/docs/pull/44533#discussion_r1935311728

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L169

Remove "nasty", "unfortunately".

- [ ] 15. Simplify "hidden" Named clients sentence

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L212

> such "hidden" Named clients go by the linked Typed client's type name

-->

> such hidden Named clients go by the Typed client's type name

- The addition of "linked" confused me and I thought it has some special meaning. I don't think you need any adjective here to express that it's talking about the typed client from the beginning of the sentence.

- Also, I see "Named client" and "Typed client" capitalized, but also not capitalized: "typed clients". Is there a reason for it?

- [ ] 16. Reduce quotes usage

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L214

Too much quotes and I don't think they are used here correctly. If you want to emphasize unwraps, hidden, exposed, make them bold.

- [ ] 17. Clarify "AnyKey" and conjunction usage

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L237

- Is "AnyKey" some constant? Does it have API docs that could be linked? It just appears here out of nowhere.
- However, as a result, - 2 conjunctions with commas, pick one and stick with it.

- [ ] 18. Avoid overusing —

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L240

I feel like you started overusing — instead of splitting into 2 sentences, or just using comma.

- [ ] 19. Call out differences in similar examples

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L252

This is very similar to the example from the beginning of the document and I'd explicitly call out the difference and why it doesn't throw here.

- [ ] 20. Remove unnecessary quotes and unfortunate

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L257

- Quotes for global are unnecessary. It's not so-called global, it is global.
- Remove unfortunate.

- [ ] 21. Describe scenarios for opting out

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L259

You just said that it's opt-in, so opting out might not make sense here.
I'd describe scenarios when one would want to use that.

- [ ] 22. Fix typo "happen"

https://github.com/dotnet/docs/blob/3b05c5f672da7c7a32b041d9be4992e2cbd83a00/docs/core/extensions/httpclient-factory-keyed-di.md?plain=1#L291

"happen" --> happen

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.