Skip to content

Conversation

@gonzalozawa
Copy link
Contributor

  • Remove legacy solution file: LearnositySDK-VS2010.sln
  • Update to .Net Standard
  • Update Newtonsoft dependency to version 10.0.3 - This was needed when updating to .Net Standard
  • Change LearnositySDK project to a Class Library that targets .NET Standard framework 2.0
  • Update LearnositySDK project to build a Nuget package

@gonzalozawa
Copy link
Contributor Author

Hi @karoltarasiuk, I have resolved the conflicts on this branch with master. Can you please have a look now and review?
With this update, my idea is to upgrade the Nuget package (https://www.nuget.org/packages/LearnositySDK/) with this latest changes to target .NET Standard.
Please let me know your comments.

<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
<RootNamespace>LearnositySDK</RootNamespace>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

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

@gonzalozawa what did you pick the version of .NET standard based on? They recommend:

to target the lowest version of .NET Standard possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karoltarasiuk this is a great question. I've tried using a lower version of .NET Standard but the project doesn't compile and throw errors. Mainly on Remote.cs and Init.cs
The problem in that particular classes is that System.Web and System.Net aren't included in the previous version of .NET Standard. See https://stackoverflow.com/questions/30227219/how-to-use-webclient-with-netcore
Do you think I should try to fix those classes and use .NET Core functionality or should we just target .NET Standard 2.0 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, makes sense. I think that in this case we leave it as is and only come back to it if somebody complains :)

@karoltarasiuk
Copy link
Contributor

karoltarasiuk commented Dec 5, 2017

@gonzalozawa I think all we need then is a squash and merge.

@gonzalozawa gonzalozawa merged commit 9b8dcb0 into master Dec 7, 2017
@gonzalozawa gonzalozawa deleted the LRN-17472 branch December 7, 2017 21:09
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.

3 participants