Migration from .NET 3.5 to .NET Core 2. #34
Conversation
ActivityDefinition has been extended to include: interactionType correctResponsesPattern choices scale target step This as been based on the work carried out by Paul Carpenter and is my first GitHub submission.
Activity definition
|
Please get this merged. This is very important for all of us going forward. |
There was a problem hiding this comment.
Sorry for the delay on this @peturingi! We've been trying to find the time to better tend to our OSS repos, and hopefully I will be able to start weighing in on these more in the near future.
A few things:
- It seems like this PR and ActivityDefinition InteractionType #35 have a lot of shared commits that span between updating this project to .NET Core and extending ActivityDefinition. These are two totally different pieces, so let's try and keep them separate. My gut is to keep Migration from .NET 3.5 to .NET Core 2. #34 for the .NET Core discussion and use ActivityDefinition InteractionType #35 to work on extending ActivityDefinition, but you can handle it however you want.
- The three merge commits are unnecessary. Please rebase your current branch instead, it keeps the commit history clean and easily navigable.
- Messing around with Rider's files isn't relevant to this. Happy to have them in the repo, but they're not part of the work to upgrade to .NET Core.
- There are two commits that just edit the README, neither of which have meaningful commit messages. You could probably just squash all those changes into c09c088, or make a separate commit that just updates documentation.
We would have to release a new major version if we were to go down this path, and customers currently running on .NET 4.5 and below would not be able to move forward with us (I am admittedly not super familiar with the cross-compatibility details here, someone please correct me if I'm wrong on this). This is particularly troublesome for us because one of our products that relies on this library has a minimum supported .NET framework of 4.5, so if we changed this library to .NET Core, we would have to in turn raise the minimum framework for our dependent application.
I don't really want to leave older frameworks in the dust, but I also don't want to worry about supporting two branches for each target. We might try to get some of the currently pending issues into master first, and then we can do a new version targeting .NET Core 2. That way we don't leave apps on older frameworks without anything, but I guess that depends on how many people using this library with .NET Framework 3.5 plan on updating anytime soon.
I want to support .NET Core applications, but we might need to find a less disruptive way to go about that. I think that moving to .NET Standard 1.1 might be one way forward since it will let us keep the 4.5 minimum of our current product, but it will allow .NET Core 2 applications to use this repo. Anyone running a Framework version below 4.5 would have to update, but the usage numbers in that range might be low enough that it's doable.
I'll also talk a bit around here and see how flexible we are about the minimum support framework. If we can bump that up to 4.6.1 and we're okay with breaking this library for 4.5 and below, then migrating to .NET Core 2 or .NET Standard 2.0 could be a real possibility.
| @@ -128,16 +98,9 @@ | |||
| <Compile Include="Properties\AssemblyInfo.cs" /> | |||
There was a problem hiding this comment.
Why is this file still present in the csproj file if you deleted it above? When I try to build your branch locally (dotnet --version is 2.0.3), one of the errors I get is that this file doesn't exist. Do you get any errors like that when you build this project?
| @@ -1,26 +0,0 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Why did you delete the entire Doc directory? Granted, they're a little slim right now, but we still want to be able to easily add documentation in the future. Whenever we get there, we would have to recreate all of this.
|
@reedmclean It should be trivial to make the code compile both .NET 4.5 and .NET Core assemblies. We've done it in the past quite easily. |
|
@JohnGalt1717, that's good to hear! Would we have to release two builds with different targets, or would one build work in both 4.5 and .NET Core applications? |
|
@reedmclean All that you have to do is add multiple targets like this:
Then Setup property groups to define compiler variables for anything that is unique between the two (don't think there is but...) You can also put your nuget packages in each of those as package references so you get the right ones for the right assemblies. Then when you build you'll end up with 2 directories in the bin folder and you can nuget publish and it will show as targeting both. In code you can use #if(Variable) #else #12 If we can back off some of the stuff and use HttpClient this could target .NET Standard 1.6 which is compatible directly with .net 4.5.2 and then it would just work without any of the targeting. But Targetting might be cleaner. |
This is a migration from .NET 3.5 to .NET Core 2.
The migration was performed due to a need to use TinCan.NET on a ASP.NET Core project which runs on Linux.
Creating this pull request as I rather not have to maintain a fork to support .NET Core. Please be explicit and reject this request in writing should you not want to support .NET Core.
Build confirmed to work on macOS, Linux and Windows.
All tests pass except one, the one failing fails because TinCan.NET does not support xapi 1.0.3 (see #27)