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.

Sign up for GitHub

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

Jump to bottom

Allow table/column comments to be specified in the model #15767

Merged
merged 1 commit into from
Jun 17, 2019
Merged

Allow table/column comments to be specified in the model #15767

merged 1 commit into from
Jun 17, 2019

Conversation

Muppets
Copy link
Contributor

@Muppets Muppets commented May 21, 2019

Summary of the changes

  • Added HasComment extensions to table and column
  • Added SQL server support for comments in extended properties

Fixes #15037

@Muppets Muppets force-pushed the 15037-Allow-table/column-comments-to-be-specified-in-the-model branch 2 times, most recently from f437a8c to 7fd255a Compare June 2, 2019 16:08
@bricelam
Copy link
Contributor

bricelam commented Jun 3, 2019 •
edited
Loading

We've also had a request to add these by-convention based on DescriptionAttribute... But we should discuss with the team in a design meeting before implementing it.

@AndriySvyryd
Copy link
Member

You also need to add a check to RelationalModelValidator.ValidateSharedTableCompatibility and RelationalModelValidator.ValidateSharedColumnsCompatibility

@Muppets Muppets force-pushed the 15037-Allow-table/column-comments-to-be-specified-in-the-model branch from e4336a2 to 1578006 Compare June 8, 2019 11:42
@Muppets
Copy link
Contributor Author

Muppets commented Jun 8, 2019

@AndriySvyryd @bricelam This should be good to go. Let me know if I've missed anything else.

@Muppets Muppets force-pushed the 15037-Allow-table/column-comments-to-be-specified-in-the-model branch 2 times, most recently from faddf11 to b1be679 Compare June 11, 2019 18:04
AndriySvyryd
AndriySvyryd reviewed Jun 15, 2019
View reviewed changes
src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs Outdated
@@ -249,6 +249,19 @@ protected virtual void ValidateDefaultValuesOnKeys([NotNull] IModel model, [NotN
otherKey.Properties.Format()));
}

var currentComment = entityType.GetComment() ?? "";
var previousComment = nextEntityType.GetComment() ?? "";
Copy link
Member

Choose a reason for hiding this comment

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

We can skip validation on null comments, just store the first non-null one and compare every other one against it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like this?

@Muppets Muppets force-pushed the 15037-Allow-table/column-comments-to-be-specified-in-the-model branch from 937f79a to 3da02b2 Compare June 16, 2019 10:31
@AndriySvyryd AndriySvyryd merged commit f9d1df4 into dotnet:master Jun 17, 2019
@AndriySvyryd
Copy link
Member

Thanks for your contribution!

@ajcvickers ajcvickers mentioned this pull request Jun 20, 2019
Closed
@Muppets Muppets deleted the 15037-Allow-table/column-comments-to-be-specified-in-the-model branch February 2, 2020 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndriySvyryd AndriySvyryd AndriySvyryd left review comments

@bricelam bricelam bricelam requested changes

Assignees

@AndriySvyryd AndriySvyryd

Labels
providers-beware
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow table/column comments to be specified in the model
3 participants
@Muppets @bricelam @AndriySvyryd