Skip to content

Conversation

@adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Jul 24, 2024

In the first commit (7db91c1), I've introduced a set of extensions that don't use BinaryFormattedObject at all, just the SerializationRecord from System.Formats.Nrbf.

Then I've realized that BinaryFormattedObject.Deserialize is not used by the product anymore, so I've removed it and everything related to parsing. All I kept is the writing logic and its tests.

This was possible because the deserializer that @JeremyKuhne has authored was moved to dotnet/runtime in dotnet/runtime#102379.

Microsoft Reviewers: Open in CodeFlow

@adamsitnik adamsitnik requested a review from a team as a code owner July 24, 2024 15:55
{
return NrbfDecoder.Decode(stream, leaveOpen: true);
}
catch (Exception ex) when(ex is ArgumentException or InvalidCastException or ArithmeticException or IOException)
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I wanted to keep the same exception handling:

catch (Exception ex) when (ex is ArgumentException or InvalidCastException or ArithmeticException or IOException)
{
// Make the exception easier to catch, but retain the original stack trace.
throw ex.ConvertToSerializationException();
}
catch (TargetInvocationException ex)
{
throw ExceptionDispatchInfo.Capture(ex.InnerException!).SourceException.ConvertToSerializationException();
}

@adamsitnik adamsitnik requested a review from JeremyKuhne July 24, 2024 17:45
@codecov
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 76.04167% with 69 lines in your changes missing coverage. Please review.

Project coverage is 74.79155%. Comparing base (8b0150c) to head (80ba212).

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11745         +/-   ##
===================================================
+ Coverage   74.69499%   74.79155%   +0.09655%     
===================================================
  Files           3045        3021         -24     
  Lines         630638      629289       -1349     
  Branches       46862       46689        -173     
===================================================
- Hits          471055      470655        -400     
+ Misses        156224      155274        -950     
- Partials        3359        3360          +1     
Flag Coverage Δ
Debug 74.79155% <76.04167%> (+0.09655%) ⬆️
integration 17.96077% <0.00000%> (+0.09666%) ⬆️
production 47.75240% <74.25373%> (+0.08604%) ⬆️
test 96.98561% <100.00000%> (-0.00003%) ⬇️
unit 44.86675% <74.25373%> (+0.07099%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Thanks for updating this!

@JeremyKuhne JeremyKuhne merged commit 6911080 into dotnet:main Jul 24, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0 RC1 milestone Jul 24, 2024
@adamsitnik adamsitnik deleted the removeSerializer branch July 25, 2024 09:51
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2024
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.

2 participants