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

Add public APIs to configure shared type entities #21552

Merged
merged 1 commit into from
Jul 18, 2020

Conversation

smitpatel
Copy link
Member

Resolves #9914

src/EFCore/ModelBuilder.cs Outdated Show resolved Hide resolved
src/EFCore/ModelBuilder.cs Outdated Show resolved Hide resolved
src/EFCore/ModelBuilder.cs Outdated Show resolved Hide resolved
src/EFCore/ModelBuilder.cs Outdated Show resolved Hide resolved
lajones
lajones previously requested changes Jul 7, 2020
Copy link
Contributor

@lajones lajones left a comment

Choose a reason for hiding this comment

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

A few updates requested.

Also I don't see the changes for UsingEntity() and the generic "this is not the API you are looking for" exception. When you say this "resolves" 9914, is there still more work to come?

@smitpatel
Copy link
Member Author

is there still more work to come?

yes, therefore it is WIP. I wanted to get early eyes from @AndriySvyryd on this before I start adding overloads, but I just added overloads for OwnsOne/Many, HasOne/Many. UsingEntity is still pending. Still writing few more tests for presently added overloads.

@AndriySvyryd
Copy link
Member

Looking good so far

@lajones
Copy link
Contributor

lajones commented Jul 8, 2020

@smitpatel As a general question, if a user calls the STET version of an API with a name and type of an existing non-STET Entity Type will you attempt to use that Entity Type, or error? If you use it will that put that Entity Type's CLR type into the list of shared CLR types?

@smitpatel
Copy link
Member Author

As a general question, if a user calls the STET version of an API with a name and type of an existing non-STET Entity Type will you attempt to use that Entity Type, or error? If you use it will that put that Entity Type's CLR type into the list of shared CLR types?

Error.

@AndriySvyryd
Copy link
Member

We should discuss whether to follow the same pattern as with weak types: The first time Entity(type, name) is called it defines a normal entity type, but when it's called with the same type again that's when it becomes a STET.
Just from the shape of the new API it's not clear that the user intended to define a STET

@AndriySvyryd
Copy link
Member

See team decision in #9914 (comment)

@smitpatel smitpatel changed the base branch from master to release/5.0-preview8 July 16, 2020 21:25
@smitpatel
Copy link
Member Author

This is just rebase & squash on latest. Will move out of draft once I update the PR for design meeting decision and ready for review.

@smitpatel smitpatel marked this pull request as ready for review July 16, 2020 23:46
@smitpatel
Copy link
Member Author

Updated.

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.

Shared-type entities (part of property bag entities)
3 participants