Add relational metadata extensions for conventions. #15667
Conversation
AndriySvyryd
force-pushed
the
Issue214_Part5
branch
from
89ff11a to
8fd6c7d
Compare
src/EFCore.Relational/Extensions/RelationalPropertyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
|
|
||
| return this; | ||
| } | ||
|
|
||
| IConventionDbFunctionBuilder IConventionDbFunctionBuilder.HasName(string name, bool fromDataAnnotation) |
There was a problem hiding this comment.
Should these and other explicit implementations have API docs?
There was a problem hiding this comment.
No, they cannot be accessed other than through the interface
There was a problem hiding this comment.
Right, but what do the generated API docs look like?
There was a problem hiding this comment.
I think these members just don't appear there
There was a problem hiding this comment.
There was a problem hiding this comment.
It looks like Roslyn doesn't catch missing docs here, but SA1600 might...
src/EFCore.Relational/Metadata/Conventions/Internal/StoreGenerationConvention.cs
Show resolved
Hide resolved
src/EFCore.Relational/Migrations/Operations/Builders/CreateTableBuilder.cs
Outdated
Show resolved
Hide resolved
...EFCore.SqlServer/Metadata/Conventions/Internal/SqlServerValueGenerationStrategyConvention.cs
Outdated
Show resolved
Hide resolved
| @@ -137,7 +137,7 @@ public static IConventionEntityType RootType([NotNull] this IConventionEntityTyp | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Gets the primary or alternate key that is defined on the given property. Returns null if no key is defined | |||
| /// Gets the primary or alternate key that is defined on the given property. Returns <c>null</c> if no key is defined | |||
There was a problem hiding this comment.
My understanding from @bricelam is to not use c or code elements for this kind of thing.
There was a problem hiding this comment.
But that's the point, these words are not in English, by making them standout the reader won't subconsciously treat them as part of the sentence.
There was a problem hiding this comment.
I don't feel strongly about it.
There was a problem hiding this comment.
I don't think it's important enough to fix in all existing comments
There was a problem hiding this comment.
I suspect a lot of it would be taken care of by one search-replace, and I do agree with @AndriySvyryd that it helps readability (but no super strong feelings either).
AndriySvyryd
force-pushed
the
Issue214_Part5
branch
from
8fd6c7d to
e10c3f6
Compare
AndriySvyryd
force-pushed
the
Issue214_Part5
branch
4 times, most recently
from
679d021 to
70206fb
Compare
Refactor relational metadata extensions to be directly defined on the metadata objects. Move conflict resolution between store generated configuration to a convention. Explicit conflicting configuration will now result in an exception. Mark UseSqlServerIdentityColumn() as obsolete in favor of ForSqlServerUseIdentityColumn() Part of #214
AndriySvyryd
force-pushed
the
Issue214_Part5
branch
from
70206fb to
963f1bc
Compare
ghost
deleted the
Refactor relational metadata extensions to be directly defined on the metadata objects.
Move conflict resolution between store generated configuration to a convention. Explicit conflicting configuration will now result in an exception.
Part of #214
Note that the PR is smaller than the new query pipeline PR, so should be easier to review.