search

keenlabs / keen-sdk-net

A .NET SDK for the Keen IO API
MIT License
37 stars 24 forks source link

Create initial, minimal csproj targeting netstandard2.0 #60

Closed baumatron closed 7 years ago

baumatron commented 7 years ago

This should be a new project that targets netstandard2.0. This will be built out as the new netstandard2.0 compliant project. We'll bring (copy) features into it one at a time from the existing codebase, so this should be located in a sibling directory among the existing SDK projects. This should build on any platform that supports netstandard2.0. A new test project should also be created for this library targeting netstandard2.0.

Pothulapati commented 7 years ago

Hello, I just forked the repo and I added a .net standard 2.0 project into the main directory. It has no files in it yet. I will also be adding the test project. After that, Is this issue completed? Or are there any steps to do?

masojus commented 7 years ago

Hi @Pothulapati, thanks for getting involved! Between this issue and Issue #74 that would be a good start. I was hoping to include a little bit of actual code as part of fixing these two issues. My thoughts were to get some of the helper classes in there and building, then have a few tests for that code in the test project. Part of the reason for this is I'm hoping Moq works with netstandard2.0 (NuGet seems to indicate it does) so that going forward porting tests will be easier.

If we get this far, I'd like to go off and get CI and try to get code coverage running for this new netstandard-based PCL (See PR #94 and Issues #71 and #72). Then as we iterate more in the next two weeks maybe we can get some automated feedback and prevent regression as we add more pieces.

Some classes that might be easy to port on the first pass just to prove the setup might be these: IProjectSettings.cs, ProjectSettingsProvider.cs, CachedEvent.cs, KeenConstants.cs, and others that don't really do a lot on their own, and don't drag in large portions of the more complex parts of the project. For extra credit, maybe include IEvent.cs and Event.cs which does more and requires IProjectSettings and IKeenHttpClientProvider, the latter of which at first I would just mock and not try to port the real implementation since it drags in all the HttpClient-related code which tends to change across platforms. If you get this far you can look at usage of TestKeenHttpClientProvider for inspiration.

masojus commented 7 years ago

https://docs.microsoft.com/en-us/dotnet/core/porting/libraries

Pothulapati commented 7 years ago

@masojus What all frameworks should the .Net Standard Library target?

masojus commented 7 years ago

I would say we just start with a single target and the broadest API surface, so <TargetFramework>netstandard2.0</TargetFramework> and then later if things go well porting the existing code we can try to relax that to a lower netstandard version and try multi-targeting to also support something like .NET Framework 4.5 or 4.6+.

Pothulapati commented 7 years ago

@masojus For the Tests, It's fine to go with a xunit .net core test project right?.I will be submitting a PR once the tests are done.

masojus commented 7 years ago

The rest of the solution uses NUnit. Is there a specific reason to switch to xUnit? Does the NUnit Visual STudio runner (as of version 3.8.0) not work? I didn't personally make the choice to use NUnit, but in terms of not causing churn without reason I'd opt for sticking to what's already being used. http://www.alteridem.net/2017/05/04/test-net-core-nunit-vs2017/

masojus commented 7 years ago

I'm going to call this closed via PR #97 and commit 422892e