fsprojects / Fleece

Json mapper for F#
http://fsprojects.github.io/Fleece
Apache License 2.0
199 stars 31 forks source link

Using System.Json 4.5.0 #32

Closed wallymathieu closed 6 years ago

wallymathieu commented 6 years ago

This fixes #29 This fixes #49

wallymathieu commented 6 years ago

Well, since I can't run the tests locally the "This fixes #29" was premature. However, it does compile, so someone, perhaps @harrisonmeister, could take it further and make a new PR.

harrisonmeister commented 6 years ago

@wallymathieu - I'm a bit busy atm but will certainly try take a look at your changes and will raise a PR if I can get it working.

wallymathieu commented 6 years ago

The prio is probably low, so no stress.

harrisonmeister commented 6 years ago

@wallymathieu - this compild and ran locally for me (i needed the path to msbuild in my PATH env variable). I couldnt find a way to get the tests to run within VS2017, and they dont appear in the build.bat script.

@gusty - are there any tips to getting the tests to run?

harrisonmeister commented 6 years ago

Ah, saw this in the appveyor script. dotnet run --project .\Tests\Tests.fsproj --framework net461 😆

wallymathieu commented 6 years ago

@harrisonmeister I've tried to fix some of the issues, could you look at my fixes? Perhaps suggestions for improvements?

gusty commented 6 years ago

@wallymathieu Maybe you can do a [WIP] PR so @harrisonmeister can reference the package in his project and test it (via Appveyor artifact).

wallymathieu commented 6 years ago

I've changed the title and added an #if SYSTEMJSON

wallymathieu commented 6 years ago

Looks like some of the behavior is intentional. I tried adding a few tests, but noticed that there are some tests of the existing behavior: https://github.com/dotnet/corefx/pull/32418

gusty commented 6 years ago

@harrisonmeister @BlythMeister I just released a new version with this PR.

It will live now in its own namespace and its nuget will be this: https://www.nuget.org/packages/Fleece.SystemJson

Notice there are some breaking changes (mainly the namespace and the dll), but there are also lots of new features. See the release notes.

In case you decide to migrate to this version, feedback will be much appreciated.

Otherwise you can stick to old Fleece nuget, v. 0.6.0, but it won't be further maintained.

harrisonmeister commented 6 years ago

@gusty - thanks for this. We try to update our nuget oss dependencies often typically once a month. We have just finished our last one a couple of days ago, so I'll try to get this new package in with our next months release and let you know how we get on.

Thanks, Mark

gusty commented 6 years ago

Thanks @harrisonmeister so based on the release notes when you migrate to Fleece.SystemJson 0.7.0 all you'll have to do is:

Hopefully it works fine for you !

harrisonmeister commented 6 years ago

Hi @gusty - no worries, thanks for the update! 👍