Review conditional code #1129
Review conditional code #1129
Conversation
We had a bunch of conditional code on frameworks that are no longer in support. Clean them up.
> SYSLIB0051: 'Exception.Exception(SerializationInfo, StreamingContext)' is obsolete: 'This API supports obsolete formatter-based serialization. It should not be called or extended by application code.' (https://aka.ms/dotnet-warnings/SYSLIB0051)
This was previously only for NET40, but actually ISynchronizeInvoke is available in all supported target frameworks now, and there's no reason to exclude it in any of them.
These were conditioned on a target we no longer build.
|
@chrbauer would be great if you could take a look over these changes. |
| @@ -82,10 +82,9 @@ public async Task AsyncWithCancellationToken() | |||
| await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await server.ReceiveStringAsync(source.Token)); | |||
| } | |||
|
|
|||
| #if NETCOREAPP3_1 | |||
|
|
|||
| #if NETSTANDARD || NET | |||
There was a problem hiding this comment.
I still get tripped up by pragma defines but this pragma only enabled the test on net5+ (dotnet test 4.7.2 does not pick it up according to the CI logs)
I imagine this pragma was part of when certain targets didn't support IAsyncEnumerable? With netstandard2.0, net4.7.2 and net8+ supporting it, I imagine this pragma can removed let the tests run always?
There was a problem hiding this comment.
It is not working out the box. The IAsyncEnumerable it not defined in net472. But maybe it could work with an "polyfill" nuget as suggested here: https://stackoverflow.com/questions/56651472/does-c-sharp-8-support-the-net-framework
There was a problem hiding this comment.
This works: chrbauer@bead6f8
There was a problem hiding this comment.
@chrbauer want to file that as a PR? Seems like a good change.
| @@ -536,7 +533,7 @@ public void RemoveReferences(int amount) | |||
| /// Override the Object ToString method to show the object-type, and values of the MsgType, Size, and Flags properties. | |||
| /// </summary> | |||
| /// <returns>a string that provides some detail about this Msg's state</returns> | |||
| public override string ToString() | |||
| public override readonly string ToString() | |||
There was a problem hiding this comment.
Resharper complains about the order of modifiers. Should be public readonly override string ToString()
There was a problem hiding this comment.
I think the issue that we don't specify a .editorconfig file that both Roslyn and JetBrains tooling can use to agree upon certain style things in the code.
Follows #1128