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

API review: Obsolete ForXxx methods in favor of shorter names #16735

Merged
merged 1 commit into from
Jul 26, 2019

Conversation

ajcvickers
Copy link
Member

Fixes #16686

@ajcvickers
Copy link
Member Author

@smitpatel @roji Ping for review.

@@ -97,7 +97,7 @@ public static class CosmosEntityTypeBuilderExtensions
/// <param name="entityTypeBuilder"> The builder for the entity type being configured. </param>
/// <param name="name"> The name of the parent property. </param>
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
public static OwnedNavigationBuilder ForCosmosToProperty(
public static OwnedNavigationBuilder ToJsonProperty(
Copy link
Member

Choose a reason for hiding this comment

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

Nice solution.

@roji
Copy link
Member

roji commented Jul 26, 2019

Now I'll go do the same for EFCore.PG...

@roji
Copy link
Member

roji commented Jul 26, 2019

Question: is it intentional to leave the lower-level metadata API for the provider prefix? We now have IsClustered in SqlServerIndexBuilderExtensions, calling into SetSqlServerIsClustered in SqlServerIndexExtensions.

@ajcvickers
Copy link
Member Author

@roji I left them because we didn't decide to change them and there may be different considerations. Also, I care less about the lower-level ones. However, we could create an issue to discuss with @AndriySvyryd when he is back.

@ajcvickers ajcvickers merged commit 6069fe8 into master Jul 26, 2019
@ghost ghost deleted the PaintingTheForthBridge0724 branch July 26, 2019 15:26
@roji
Copy link
Member

roji commented Jul 26, 2019

@ajcvickers opened #16773 to discuss.

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.

Rename ForProviderXxx... methods
2 participants