-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Mitigate AddProvider verse AddEntityFrameworkProvider method confusion #25451
Conversation
/// <param name="inMemoryOptionsAction"> An optional action to allow additional in-memory database specific configuration. </param> | ||
/// <param name="optionsAction"> An optional action to configure the <see cref="DbContextOptions" /> for the context. </param> | ||
/// <returns> The same service collection so that multiple calls can be chained. </returns> | ||
public static IServiceCollection AddInMemoryDatabase<TContext>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooo I like how this reads 😃
We could also have the analyzer flag usages of AddEntityFrameworkProvider - opened #25455 to track this. |
src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
8ef9811
to
530c2fa
Compare
src/EFCore.Cosmos/Extensions/CosmosServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
Note from triage: remove |
@ajcvickers that's a shame as the in-memory database can be a good candidate during tutorials when the complexity of initializing and managing a storage provider adds more complexity/concepts to the learning stage than is desired. In-memory being self-initializing and requiring no separate acquisition or setup is appealing in these scenarios. |
Why would we remove this? It's so simple to get started and avoids the need for migrations until required. |
Because it's not a real database. A lot of users expect it to have relational semantics or being production-ready. InMemoryDatabase has a very limited scope and we should steer them towards SqlServer LocalDB or Sqlite in-memory. Microsoft has been criticized for building toy apps in tutorials using techniques that aren't acceptable in production-ready code, InMemoryDatabase certainly falls into this category. |
It's already in another package you have to explicitly reference. Not adding this method doesn't do anything. I think we should add the method. You can continue to push people to SQLite but don't neuter the package arbitrarily |
By hiding the infrastructure method from Intellisense and adding docs. Also adds Cosmos and In-Memory sugar. Part of #24743
530c2fa
to
91a6bb9
Compare
Split out the in-memory question to #25487 so that the rest can be merged. We can continue discussion there. |
By hiding the infrastructure method from Intellisense and adding docs.
Also adds Cosmos and In-Memory sugar.
Part of #24743