HttpClientFactory doc improvements
- 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
Assessment
This issue has not been assessed yet.