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

Target EF Core 2.1.1 & remove fody for nuget package #185

Closed
wants to merge 6 commits into from

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented Jul 16, 2019

In order to provide compatibility with .Net Core 2.1 solutions, this pull request changes the references on the main project to target EF Core 2.1.1 rather than 2.6 and also removes the unneeded (???) fody references (which require .Net Core 2.2) from the main project. The test project references remain the same, and tests pass just fine. Note that I assume the tests are still run against EF Core 2.6. No code changes are necessary. I do not believe these changes have any detrimental effects, but they are necessary in order to use this project with any EF Core 2.1 project.

I'm requesting these changes because I'm building a plugin for nopCommerce 4.1, which uses EF Core 2.1.1. With these few reference changes, so far it is working great.

@SimonCropp
Copy link
Owner

@Shane32
Copy link
Contributor Author

Shane32 commented Jul 17, 2019

  • I'm sorry, I'm not sure what fody is and didn't realize it was being used.
  • This project requires that I leave the main project references alone, so it must be EF Core 2.1.1. I already tried compiling the plugin with EF Core 2.2.6, but the main project would not load the dependency in that configuration.
  • If I were to upgrade the references, AspNetCore.App 2.1.2 depends on EF Core >= 2.1.1 < 2.2.0. So I can't only upgrade EF; I'd have to upgrade AspNetCore.App to 2.2.0+.
  • Yes, I'm a patron under the name Zbox.

It would really be convenient if you could publish the nuget package with references to EF Core >= 2.1, as that's all that is really required. All your code works perfectly with the older version of EF, and it would not break any existing applications. If this doesn't work for you, I can fork the project and recompile.

@SimonCropp
Copy link
Owner

Yes, I'm a patron under the name Zbox.

Yep. sorry for the repeated question

already tried compiling the plugin with EF Core 2.2.6, but the main project would not load the dependency in that configuration.

would a binding redirect work for you?

can u update nopCommerce to the latests EF? i really thing u should try to stay reasonably up to date. for example i would assume you want this memory leak to not effect you dotnet/efcore#15173

@Shane32
Copy link
Contributor Author

Shane32 commented Jul 18, 2019

We have other closed source plugins for nopCommerce, likely locking us into that particular major version of nopCommerce's dependencies. Ignoring that, while the goal is to have the latest dependencies whenever possible, this is not always feasible due to our limited resources. At the moment, we plan to skip nopCommerce 4.2 (which would already have the newer dependencies) and upgrade to 4.3 after it is released, which I expect to be based on .Net Core 3. Unfortunately, this means we cannot take advantages of a great deal of enhancements, of both security and speed, which are available in nopCommerce 4.2 and its dependencies (such as EF).

I can explore utilizing a binding redirect.

@SimonCropp
Copy link
Owner

if any version of nopCommerce is locking you into an older, and buggy, version of EF (or any other dependency) then that is a significant problem you need to address. for example, what happens if some security patches are released?

As for why i always release new versions against the latest stable of any upstream...

I manage a significant number of packages https://www.nuget.org/profiles/simoncropp. to enable this i need to optimize for smallest overhead per project. always releasing new versions against the latest stable of any upstream is one such mechanism. the reason being

  • having a lower dependency range means i am more likely to have people raise bugs and ask for help when the true underlying cause is they are experiencing a bug in an older version of said dependency.
  • having a lower dependency makes it is very difficult verify, with any level of confidence, that my project works against all the versions within the range. for example, in this PR you left the test project targeting the latest EF. So we know tests pass against that version. but we have no way to know if it works against the 10 versions in between the current and 2.1.1
  • having "different version dependency rules per csproj in a solution" makes it difficult to reason about when doing updates. eg if you have samples projects, test projects, documentation snippets projects, and main projects. having to think about when to change what version adds effort. it would be manageable on the projects in one solution, but not when managing 400+csproj (the number of current csproj i have in my code directory).

@Shane32
Copy link
Contributor Author

Shane32 commented Jul 19, 2019

I understand if your goals do not meet my needs. Feel free to close the PR; I will just have to maintain a fork (or hopefully a simple binding redirect). No hard feelings! A few comments below:

Personally, I feel that there are always bugs in every version of every dependency: known/patched bugs, and unknown bugs. Everyone must keep a schedule for updating their projects; this is why there are LTS releases and interim releases of major frameworks and operating systems. I'm on an old version, and I know that.

For my desktop projects, I like to keep them running on as old as a framework as will support the software, upgrading when a new feature necessitates a change, so I can target as wide of a user-base as possible. For instance, I have a image printing program that runs on .Net Framework 2.0. It still works perfectly and we use it often. Hopefully if there was any bugs, Microsoft patched them in the newer versions of .Net, which would carry-over to my software so long as that version is installed on the end user's computer. But the program will run on Windows 98 just as well as Windows 10.

.Net Core also runs the latest installed patch version -- see https://docs.microsoft.com/en-us/dotnet/core/versions/selection . And as .Net Core 2.1 is actually a LTS version, it will be supported and patched longer than the current .Net Core 2.2 version -- see https://dotnet.microsoft.com/platform/support/policy/dotnet-core .

So for a public package such as this, I personally would target as old a version of EF as possible (and test against that version), and let your users decide if they wish to use a newer version of EF. When breaking changes to EF are released (such as when 3.0 drops), or major features are added that would provide a benefit to your plugin, then of course you would upgrade the dependency. But that's just me, and I completely understand your point of view, particularly how it might reduce bug reports.

Anyway, thanks for listening!

@SimonCropp
Copy link
Owner

For my desktop projects, I like to keep them running on as old as a framework as will support the software

Out of curiosity are those commercial apps?

@Shane32
Copy link
Contributor Author

Shane32 commented Jul 20, 2019

Yes, all the software I write and maintain is used internally in a commercial environment. However, very little of it is desktop software anymore; mostly they are intranet web applications. The nopCommerce project is for a e-commerce business we are starting (Zbox).

@SimonCropp
Copy link
Owner

i think that is the difference here. if this project was true commercial software that i was being paid to work on, i would optimize for getting as much distribution as possible. hence target a much larger version range.

@SimonCropp
Copy link
Owner

ok i put some time into thinking about this. and i am closing it.

@SimonCropp SimonCropp closed this Aug 10, 2019
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.

2 participants