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

Move entity classes to EF-Independent project #17

Merged
merged 3 commits into from
Mar 14, 2018

Conversation

ckalan
Copy link
Contributor

@ckalan ckalan commented Mar 14, 2018

I've moved the entity classes to a separate project so that people can reference them in their model or domain projects without any requirement to reference EntityFramework. This way one can use the same model in the future with EF Core.

@ckalan
Copy link
Contributor Author

ckalan commented Mar 14, 2018

@mrahhal If you are OK with the PR, could you please upload the Model nuget package so that we can reference it in our projects?

Copy link
Owner

@mrahhal mrahhal left a comment

Choose a reason for hiding this comment

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

Sounds like a good idea.

In addition to the review comments I added that require changes, could you also rename the project from "Model" to "Models"? I'd like to follow this convention. (Make sure the folder's name is renamed too).

After those changes, I'll start releasing this as a package.

<PropertyGroup>
<TargetFramework>netstandard1.3</TargetFramework>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<Version>3.0.4</Version>
Copy link
Owner

Choose a reason for hiding this comment

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

There is a common.props file that I import in all csprojs that let me share lots of those values, and also reference a single version. So please check the other csprojs on how to edit this one to take advantage of common.props. You'll still need to add the TargetFramework element above to override that.

build.cake Outdated
DeleteDirectory("./artifacts", new DeleteDirectorySettings {
Recursive = true,
Force = true
});
Copy link
Owner

Choose a reason for hiding this comment

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

Some indentation on the last 3 lines here please. And remove the line trailing line after this.

@ckalan
Copy link
Contributor Author

ckalan commented Mar 14, 2018

I've made some changes based on your requests. Could you please check?

@@ -1,16 +1,15 @@
<Project Sdk="Microsoft.NET.Sdk">

<Import Project="../../build/version.props" />
Copy link
Owner

@mrahhal mrahhal Mar 14, 2018

Choose a reason for hiding this comment

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

"common.props" please! It includes "version.props" and adds stuff like "PackageLicenseUrl" and the like.

Copy link
Owner

@mrahhal mrahhal left a comment

Choose a reason for hiding this comment

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

Ok looks good. I can change the project's name myself as I need to do some edits in any case. I'll merge this with another branch to do my edits.

@mrahhal mrahhal merged commit a26bc00 into mrahhal:master Mar 14, 2018
@mrahhal
Copy link
Owner

mrahhal commented Mar 14, 2018

Fixing an issue with netstandard tfm usage. Package "MR.AspNet.Identity.EntityFramework6.Models" should be ready by today.

@ckalan
Copy link
Contributor Author

ckalan commented Mar 14, 2018

I didn't have time to fix those today. I see you've already merged it. Thanks!

@mrahhal
Copy link
Owner

mrahhal commented Mar 15, 2018

Released as 3.0.5, thanks a lot!

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.

None yet

2 participants