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

New doc on context pooling #2662

Merged
merged 9 commits into from
Sep 22, 2020
Merged

New doc on context pooling #2662

merged 9 commits into from
Sep 22, 2020

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Sep 17, 2020

@Rick-Anderson
Copy link
Contributor Author

@daniel-white @ROMYIM @Menighin please review

@Rick-Anderson Rick-Anderson marked this pull request as ready for review September 18, 2020 00:58
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Thanks @Rick-Anderson, good to see all these great docs!

entity-framework/core/miscellaneous/dbcontextpool.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/dbcontextpool.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/dbcontextpool.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/dbcontextpool.md Outdated Show resolved Hide resolved
<xref:Microsoft.Extensions.DependencyInjection.EntityFrameworkServiceCollectionExtensions.AddDbContextPool%2A> enables a pool of reusable `DbContext` instances. To use `DbContext` pooling, use the `AddDbContextPool` instead of `AddDbContext` during service registration:

``` csharp
services.AddDbContextPool<BloggingContext>(
Copy link
Member

Choose a reason for hiding this comment

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

As a general rule, we try to always put code samples under the samples in this repo, and reference it - this makes sure code always works and is up to date (we build the sample repo as part of the docs CI).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roji do you want me to add a project to samples that just has a main that calls startup with this code

entity-framework/core/miscellaneous/dbcontextpool.md Outdated Show resolved Hide resolved
entity-framework/toc.yml Outdated Show resolved Hide resolved
Co-authored-by: Shay Rojansky <roji@roji.org>
@roji roji changed the title New doc Connection Pooling New doc on context pooling Sep 20, 2020
@@ -7,6 +7,11 @@
"build_output_subfolder": "ef",
"locale": "en-us",
"open_to_public_contributors": true,
"xref_query_tags": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ajcvickers this change allows the repo to use xref tags like I added to the new doc
<xref:Microsoft.EntityFrameworkCore.DbContext>

Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

I think the consensus we reached in the team is that we backtick a type once when we introduce it, but remove backticks later to make the text flow better. For DbContext specifically, we generally prefer to talk about "context" - I've submitted suggestions to do so.

Apart from that it all looks good to me.

entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
entity-framework/core/miscellaneous/context-pooling.md Outdated Show resolved Hide resolved
@Rick-Anderson
Copy link
Contributor Author

Rick-Anderson commented Sep 22, 2020

I think the consensus we reached in the team is that we backtick a type once when we introduce it, but remove backticks later to make the text flow better. For DbContext specifically, we generally prefer to talk about "context" - I've submitted suggestions to do so.

Apart from that it all looks good to me.

The MS style guide is very clear on this. The first use should use <xref:type> and subsequent should all be code fenced. That's how all of docs.ms.com is supposed to be written.

Note this PR adds "xref_query_tags": [ to .openpublishing.publish.config.json to make that possible.

@ajcvickers
Copy link
Member

@Rick-Anderson The problem is that the rendering is so ugly... We really don't want our docs to be unreadable in this way. Can you point to the data that shows always fencing creates more readable docs? Or some other data-based reason that the docs are this way?

@Rick-Anderson
Copy link
Contributor Author

Rick-Anderson commented Sep 22, 2020

I think I may have misunderstood. Changing DbContext to context is fine, as context is not a type. Rarely can you substitute a type for a general term.

The problem is that the rendering is so ugly...

If you're talking about types, my guess is the consensus is fencing makes the doc look better. Perhaps your eyes need to get used to it. It's also a consistency issue with docs.ms.com. I think most writers would say it looks better with types fenced and makes the docs more readable.

On problem with replacing DbContext with context is that context is now localized, and in some languages that can be problematic.

Can you point to the data that shows always fencing creates more readable docs? Or some other data-based reason that the docs are this way?

I'll bring it up with the folks who author the style guide. They usually have sound reasons for each rule AFAIK.

Rick-Anderson and others added 2 commits September 22, 2020 06:50
Co-authored-by: Shay Rojansky <roji@roji.org>
Co-authored-by: Shay Rojansky <roji@roji.org>
@roji roji merged commit 5e401dd into master Sep 22, 2020
@roji
Copy link
Member

roji commented Sep 22, 2020

I do agree we should be consistent with general MS docs standard, the thing is that this is the way our docs are currently written pretty much across the board. As an example, see DbSet in the docs on entity types. We could obviously change everything (again) if needed, but for now it is what it is (and I think we like it).

Re localization for context, unfortunately I think that's a pretty universal problem. For example, we fence the AddDbContextPool method, but then talk about pooling (the concept) in English - the two may end up completely disconnected after localization, etc.

@Paul-Dempsey
Copy link

Paul-Dempsey commented Oct 5, 2020

"context" may be clearer, and translate better if you say: "database context", and on first use, (similar to acronyms), say "database context (DbContext)". Then it is clear that you are introducing specific terminology.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document resolving services when using AddDbContextPool New feature topic: DbContext pooling
5 participants