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

Automatically include *.edmx files in SDK-style projects #1095

Merged
merged 1 commit into from
Jul 30, 2019

Conversation

bricelam
Copy link
Contributor

Fixes #891

@bricelam bricelam requested a review from ajcvickers July 29, 2019 22:55
@bricelam
Copy link
Contributor Author

@pranavkm @javiercn Any gotchas?

@javiercn
Copy link
Member

I’d recommend having buildTransitive pros to point to buildMultitargeting prod and that to build props.

I think you added the elements correctly AFAIK I don’t remember if there were 2 flags or 1 for content, as 1 for default items and a more specific one for web content.

WRT msbuild im not sure if you need to compare against 'True' or 'true'. I always do the latter, but not sure if it matters.

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

@CZEMacLeod
Copy link

It would be useful to have an EF6 specific property to turn this behavior off. I use CodeFirst and Migrations, but also extract the target EDMX from the migration resource file and set it to be parented to the migration. This means that I can easily look at the generated model visually and check if it is correct or use it for reference, print it, share it with my team etc.
With this change (and I think something in VS2019 when it adds EDMX files now even with EF6.2) I have to effectively override this and set all my EDMX 'reference' files back to None.
Maybe we could use the standard Migrations or Migrations.* folders as an exclusion?

@bricelam
Copy link
Contributor Author

@javiercn AFAICT, buildMultitargeting is only needed to affect restore and pack. These are effectively embedded resources so they only matter when compiling for a specific framework.

MSBuild is case insensitive. I like to channel my inner VB developer when writing conditions since it's And not &&.

I'll add an off switch for edmx files similar to the existing ones. This should enable @CZEMacLeod's scenario

@javiercn
Copy link
Member

@javiercn AFAICT, buildMultitargeting is only needed to affect restore and pack. These are effectively embedded resources so they only matter when compiling for a specific framework.

That's fair. I normally do the chaining transitive -> multitargeting -> build because its safe in all situations.

MSBuild is case insensitive. I like to channel my inner VB developer when writing conditions since it's And not &&

It's fair

I'll add an off switch for edmx files similar to the existing ones. This should enable @CZEMacLeod's scenario

This is a good thing to have. The razor SDK also does it.

@bricelam
Copy link
Contributor Author

Oh I guess the other place where you need multitargeting is when you have an entry-point target. Otherwise msbuild /t:MyTarget wouldn't work on multi-targeting projects.

@bricelam
Copy link
Contributor Author

bricelam commented Jul 30, 2019

Updated. @CZEMacLeod add this to your project to prevent it from happening by default (you'll still be able to manually change them to EntityDeploy items if you want)

<PropertyGroup>
  <EnableDefaultEntityDeployItems>False</EnableDefaultEntityDeployItems>
</PropertyGroup>

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.

Include *.edmx files by default
4 participants