-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Improvements to value generation docs #2998
Conversation
60a5403
to
701e492
Compare
"MD024": { | ||
"allow_different_nesting": true | ||
}, | ||
"MD024": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dotnet/docs uses individual <!-- markdownlint disable MD024-->
statements when the old tab format causes issues instead of turning off rules entirely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an alternative new tab format the we can use instead, that doesn't have this problem?
If not, forcing these linter suppression in the doc codes seems to me more annoying/ugly than the value that MD024 brings, though that's subjective...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was supposed to be a :::
version at some point awhile ago, but I think most of the docs for that type of thing is in the https://review.docs.microsoft.com/ site that isn't available to the public
[!code-csharp[Main](../../../samples/core/Modeling/FluentAPI/ComputedColumn.cs?name=StoredComputedColumn)] | ||
## Overriding value generation | ||
|
||
Although a property is configured for value generation, in many cases you may still explicitly specify a value for it. Whether this will actually work depends on the specific value generation mechanism that has been configured; while you may specify an explicit value instead of using a column's default value, the same cannot be done with computed columns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a bit light, compared to the "Setting Explicit Values for Generated Properties" page which this PR removes (though note that some of its content went into the SQL Server docs). It seems reasonable to me, but would accept opinions to the contrary.
|
||
To provide an explicit value for properties that have been configured as value generated on add or update, you must also configure the property as follows: | ||
|
||
[!code-csharp[Main](../../../samples/core/Modeling/FluentAPI/ValueGeneratedOnAddOrUpdateWithPropertySaveBehavior.cs?name=ValueGeneratedOnAddOrUpdateWithPropertySaveBehavior&highlight=5)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the original section in "Setting Explicit Values for Generated Properties" said to use PropertySaveBehavior.Ignore which didn't work for me - PropertySaveBehavior.Save does. Also, no exception was thrown when specifying Ignore (which is what the text said).
One last thing, when the property ValueGeneratedOnAddOrUpdate()
, then overriding with explicit values works when updating (with PropertySaveBehavior.Save), but doesn't seem to work when adding...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened dotnet/efcore#23912 to track the apparent adding bug.
* Create new page on SQL Server value generation, with information on IDENTITY, seed/increment, etc. * Delete "explicit values for generated properties" page, moving relevant content to the new SQL Server page and to the fixed-up generated properties page. Closes #2999
Co-authored-by: Andriy Svyryd <[email protected]>
Co-authored-by: Andriy Svyryd <[email protected]>
4a64261
to
01d7cb4
Compare
This ended up rabbit-holing into bigger change than just #2980. @AndriySvyryd let me know what you think.
Fixes #2980
Fixes #2947
Closes #2999