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

System.Text.Json serializer #6749

Merged
merged 5 commits into from
Jun 8, 2023
Merged

System.Text.Json serializer #6749

merged 5 commits into from
Jun 8, 2023

Conversation

DavidBoike
Copy link
Member

No description provided.

@DavidBoike DavidBoike added this to the 8.1.0 milestone Jun 2, 2023
@DavidBoike DavidBoike self-assigned this Jun 2, 2023
@andreasohlund
Copy link
Member

LGTM apart from some missing API docs.

@DavidBoike DavidBoike marked this pull request as ready for review June 5, 2023 17:52
@SimonCropp
Copy link
Contributor

so will you be writing a upgrade/transition guide for NServiceBus.Json package?

will you submit a PR to NServiceBus.Json with obsolete messages?

Copy link
Contributor

@danielmarbach danielmarbach left a comment

Choose a reason for hiding this comment

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

Would it make sense to mave the JSon package reference from

  <ItemGroup Condition="'$(TargetFramework)' == 'net472'">
    <PackageReference Include="System.Memory" Version="4.5.5" />
    <PackageReference Include="System.Text.Json" Version="7.0.2" />
  </ItemGroup>

to

  <ItemGroup Label="Public dependencies">
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="7.0.0" />
    <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="7.0.2" />
    <PackageReference Include="System.Security.Cryptography.Xml" Version="7.0.1" />
  </ItemGroup>

? Then we would have the same JSON version in place for all TFMs

@DavidBoike
Copy link
Member Author

@SimonCropp

so will you be writing a upgrade/transition guide for NServiceBus.Json package?

will you submit a PR to NServiceBus.Json with obsolete messages?

Yes to all 😄

@DavidBoike
Copy link
Member Author

FYI @danielmarbach, decided not to apply the V7 package across the board. If we didn't have .NET Framework targets we would probably just not declare the dependency and use the in-box version, at least until we try to do something (like with the source generators) that requires a specific version. So if you close one eye and ignore the net472 stuff now, that's essentially what we have.

@DavidBoike DavidBoike merged commit 78b016e into master Jun 8, 2023
@DavidBoike DavidBoike deleted the system-json branch June 8, 2023 13:42
@danielmarbach
Copy link
Contributor

Agreed. Currently we also are not using anything of the version specific things so it should be fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants