- Notifications
You must be signed in to change notification settings - Fork749
Use the BinaryFormatter only on .NET FW and Mono#2470
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b696f83 to4ccde5bCompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Look good, but I'd like NITs to be fixed and the commit squashed
| @@ -33,6 +33,10 @@ static T SerializationRoundtrip<T>(T item) | |||
| { | |||
| using var buf = new MemoryStream(); | |||
| var formatter = RuntimeData.CreateFormatter(); | |||
| if (formatter == null) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It might be a good idea to make the whole test class[Obsolete] to prevent warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well, for the time being, we still by default support this functionality on .NET FW and Mono. Which warning are you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Obsolete code warning that comes from usingBinaryFormatter. You can see it on GitHub review page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
4ccde5b to1c0afe6CompareWell, without the stashing, |
lostmsu commentedOct 28, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
We could consider dropping the support for Shutdown/Initialize for .NET Core+. I don't know if there are any good scenarios for it. It is much safer to spawn a new process. |
958dd30 to3cd685eCompare| @@ -124,6 +132,12 @@ internal static void RestoreRuntimeData() | |||
| private static void RestoreRuntimeDataImpl() | |||
| { | |||
| IFormatter? formatter = CreateFormatter(); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think coming here should be an error.
Uh oh!
There was an error while loading.Please reload this page.
Shouldfix#2469