Skip to content

Add EntityTypeConfiguration<TEntity> #6989

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

Merged
merged 1 commit into from
Jan 19, 2017
Merged

Add EntityTypeConfiguration<TEntity> #6989

merged 1 commit into from
Jan 19, 2017

Conversation

rowanmiller
Copy link
Contributor

Resolves #2805

/// in <see cref="DbContext.OnModelCreating(ModelBuilder)"/>.
/// </summary>
/// <typeparam name="TEntity"> The entity type to be configured. </typeparam>
public abstract class EntityTypeConfiguration<TEntity>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not an interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the thought didn't even cross my mind... I was just doing it like we did in EF6 (eyeroll from @anpete 😁). I'll swap it to an interface.

Copy link
Contributor

@ajcvickers ajcvickers left a comment

Choose a reason for hiding this comment

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

:shipit:

@anpete
Copy link
Contributor

anpete commented Nov 10, 2016

May want to create a separate bug for auto-discovery of these, which is mentioned in the original bug.

@anpete
Copy link
Contributor

anpete commented Nov 10, 2016

Are we OK with only having a generic version?

@rowanmiller
Copy link
Contributor Author

I don't know that we want to auto-scan for these but I think an AddFromAssembly like we had in EF6 is an ok idea.

I played with having a non-generic one too - mostly so that I could have an ApplyConfiguration() method (which is super messy with just the generic version). I'm happy to add it if we think it's helpful. I decided not to because you end up with public stuff on the generic version that is distracting (public Type EntityType { get;} etc.)... but happy to add it back.

@ErikEJ
Copy link
Contributor

ErikEJ commented Nov 11, 2016

@sjh37 !!

@sjh37
Copy link

sjh37 commented Nov 11, 2016

A nice improvement. I'll make sure to add this to the EF core reverse generator.

@ErikEJ
Copy link
Contributor

ErikEJ commented Nov 11, 2016

@sjh37 Yes, it should allow you to split the files in a similar way to how you do it now

@rowanmiller
Copy link
Contributor Author

Decided in design meeting to just keep the generic version. Updated PR to use interface rather than abstract base class.

@anhphan
Copy link

anhphan commented Apr 15, 2017

Not sure keeping Generic style is a good idea, currently I have a self implement of mapping and I have something like this for registration:

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            var collections = ServiceProvider.GetServices<IModelBuilder>();
            foreach (var callback in collections)
            {
                callback.OnModelCreating(modelBuilder);
            }
        }

For auto scan, I used ApplicationParts to registry Mapping class to DI.

@Diaskhan
Copy link

Diaskhan commented Mar 6, 2018

After on cofiguring we could access to dbset of dbcontext?
How usually manipulate with dynamically loading poco objects ?

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.

9 participants