Skip to content

Partial Fix for 6674. Allow inline configuration of Navigations as they are created #20195

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Mar 8, 2020

Conversation

lajones
Copy link
Contributor

@lajones lajones commented Mar 6, 2020

WIP: Partial Fix for #6674. Allow inline configuration of Navigations as they are created.

Note: there's more to come. At the moment this just supports the WithMany(...). HasOne(...) syntax, but I know I need to do the others too.

@lajones lajones requested a review from AndriySvyryd March 6, 2020 01:03
@lajones lajones force-pushed the 20200305_Issue6674_02 branch from c7e8bdd to 1c26c4d Compare March 7, 2020 04:45
@lajones lajones changed the title WIP: Partial Fix for 6674. Allow inline configuration of Navigations as they are created Partial Fix for 6674. Allow inline configuration of Navigations as they are created Mar 7, 2020
@lajones
Copy link
Contributor Author

lajones commented Mar 7, 2020

@AndriySvyryd - this is the fix for all generic and non-generic updates to HasOne/HasMany/WithOne/WithMany except many-to-many. I'd like to get this checked in before it gets too unwieldy and continue to work on the rest separately (which includes the update to MemberInfo instead of PropertyInfo).

lajones added 2 commits March 7, 2020 15:30
…ectly

Updated WithOneBuilder calls to do nav config.
Extracted IConventionNavigationBuilder.

navigationConfiguration(
new NavigationBuilder(navigation));
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest extracting this to a common method as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into it. They all do slightly different things - navigationName vs navigationMember etc.

Copy link
Member

Choose a reason for hiding this comment

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

That's fine, you can abstract them as MemberIdentity, see WithOneBuilder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@AndriySvyryd
Copy link
Member

Add NavigationBuilder to _fluentApiTypes in ApiConsistencyTest

@lajones
Copy link
Contributor Author

lajones commented Mar 8, 2020

All comments addressed.

Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

Don't forget to squash your commits

@lajones lajones merged commit e318524 into master Mar 8, 2020
@lajones lajones deleted the 20200305_Issue6674_02 branch March 8, 2020 18:50
@lajones lajones self-assigned this Mar 8, 2020
@lajones lajones mentioned this pull request Mar 13, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants