switch to File Scope Namespace, and more #27
Conversation
AoshiW
commented
Jul 3, 2023
AoshiW
commented
- switch to File Scope Namespace
- small performance improvement in webSocket
- little changes
| @@ -1,7 +1,7 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
|
|
|||
| <PropertyGroup> | |||
| <TargetFramework>net7.0</TargetFramework> | |||
| <TargetFrameworks>net7.0</TargetFrameworks> | |||
There was a problem hiding this comment.
After reverting multiple frameworks, this can be switched back to TargetFramework, but it's not a big deal.
There was a problem hiding this comment.
I originally wanted to add a test for current LTS .NET6 but the test failed so I removed that test for now
and someone would change TargetFramework to TargetFrameworks anyway in the future, so I left it there.
|
@Syzuna @swiftyspiffy Can you please look at this? I would add it to 2.0 release. There's nothing major changing but it improved summary commands and fixes typos. I went through this whitespace moving hell for you guys and I don't see any issues. It's mostly just shifting spaces due to File Scope Namespaces what makes this really hard to review. Overal I think this is good to go. |
| { | ||
| public string Data { get; } | ||
| public Exception Exception { get; } | ||
| public string Data { get; } |
There was a problem hiding this comment.
we use Message in OnMessageEventArgs, and the type here is still string. Should this be Message instead of Data?
There was a problem hiding this comment.
I don't have a strong opinion on it, so I can change it if you want
There was a problem hiding this comment.
I would keep it consistent yeah. either change both to Data or both to Message
Co-authored-by: neon-sunset <[email protected]>
