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

fix infinite loop in new datetime converter, fixes issue #463 #465

Merged
merged 1 commit into from
Feb 19, 2019
Merged

fix infinite loop in new datetime converter, fixes issue #463 #465

merged 1 commit into from
Feb 19, 2019

Conversation

softlion
Copy link
Contributor

@softlion softlion commented Feb 19, 2019

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

fixes #463

What is the current behavior? (You can also link to an open issue here)

The problem was that, as the default ResolveContract() calls CreateContact(), in your CreateContact() you can't call ResolveContract() on the embedded contractResolver, as it will also (often) inherit from DefaultContractResolver.

So it loops.

What is the new behavior (if this is a feature change)?

I moved the code in ResolveContract instead, which make the issue disappear. It won't loop anymore!

I also check if the contract is already overriden by the other resolver. In this case let it handle the conversion.

Does this PR introduce a breaking change?

No.

Please check if the PR fulfills these requirements

Other information:

@glennawatson glennawatson merged commit 7a11834 into reactiveui:master Feb 19, 2019
@@ -9,6 +9,7 @@
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
<PackageReference Include="Microsoft.Reactive.Testing" Version="4.0.0" />
<PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="1.1.13" />
Copy link
Contributor

Choose a reason for hiding this comment

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

NuGet packages are inherited by other included project references in csproj 2017. So this isn't really needed but figured I would get your NuGet out for you then change it after the fact. Reason why we don't want to explicitly set it is because we want the versions just to be set in one place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact it was needed because it was just crashing on execution with the "missing sqlite3 dll" message. I suppose it has specific tests for net47 not included by inheritance. I have the latest version of VS so ... it's not the problem.

@glennawatson
Copy link
Contributor

Passing locally and on the CI so will keep an eye on it. Likely be a native dll not being copied across.

@glennawatson
Copy link
Contributor

glennawatson commented Feb 19, 2019

https://www.nuget.org/packages/akavache/6.3.6

An asynchronous, persistent key-value store for desktop and mobile applications on .NET

@lock lock bot locked and limited conversation to collaborators Jun 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inifinite loop in the DateTime resolvers.
2 participants