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

cln: add JsonConverter for handling enum #225

Merged
merged 2 commits into from
Aug 16, 2022
Merged

Conversation

joemphilips
Copy link
Owner

Unlike Newtonsoft.Json, System.Text.Json does not
respect EnumMemberAttribute. (see: dotnet/runtime#31081)
This caused an issue when sending request to c-lightning,
when you use System.Text.Json for serialization lib, and there
is a enum value in the request field, the field will be serialized into
an int, instead of a string. Thus causing RPC error.

This commit fixes it by using JsonStringEnumConverterEx
(which is taken from an issue comment of issue above).

Unlike Newtonsoft.Json, System.Text.Json does not
respect EnumMemberAttribute. (see: dotnet/runtime#31081)
This caused an issue when sending request to c-lightning,
when you use System.Text.Json for serialization lib, and there
is a enum value in the request field, the field will be serialized into
an int, instead of a string. Thus causing RPC error.

This commit fixes it by using `JsonStringEnumConverterEx`
(which is taken from an issue comment of issue above).
@joemphilips joemphilips force-pushed the fix_json_converter_bug branch from 22aa6b8 to e2ae66b Compare August 16, 2022 10:48
@knocte
Copy link
Collaborator

knocte commented Aug 16, 2022

I see you're trying to find the packages dir where nuget packages get downloaded. I recently had trouble with this because apparently M$ has changed behaviour; to go back to the old behaviour of creating a packages subfolder in the repo itself when restoring, you can create a NuGet.config file with this content:

<configuration>
    <config>
        <add key="globalPackagesFolder" value=".\packages\" />
    </config>
</configuration>

@joemphilips joemphilips force-pushed the fix_json_converter_bug branch from e2ae66b to fa9a3b3 Compare August 16, 2022 11:34
@joemphilips
Copy link
Owner Author

joemphilips commented Aug 16, 2022

@knocte Thanks, but it seems this is a different issue.

It seems that fsdocs fails to find a path to 3rd party dll, (in this case NBitcoin)
ref: fsprojects/FSharp.Formatting#616

I don't know why but it works in local, even when I remove the nuget cache with dotnet nuget locals global-packages -c

I should probably change the publish workflow to more manual way, as I have to do it anyway for #224

@joemphilips
Copy link
Owner Author

It seems that fsdocs fails to find a path to 3rd party dll, (in this case NBitcoin)
ref: fsprojects/FSharp.Formatting#616
I don't know why but it works in local, even when I remove the nuget cache with dotnet nuget locals global-packages -c

@knocte hmm, it works fine when I use an old version of .NET SDK. Maybe the root cause is same with yours.
Anyway I should report this to fsdocs repo.

It seems that fsdocs has a bug in the latest .NET SDK (6.0.400).
It fails to build a API doc for a package which uses 3rd party lib.
Avoid this by specifying an older version explicitly for CI build.
@joemphilips joemphilips merged commit 5303ac7 into master Aug 16, 2022
@joemphilips joemphilips deleted the fix_json_converter_bug branch August 16, 2022 13:07
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.

2 participants