Repository navigation
Conversation
| <PackageReference Include="SocketIOClient" Version="4.0.5" /> | ||
| <PackageReference Include="System.Drawing.Common" Version="8.0.22" /> | ||
| <PackageReference Include="System.Text.Json" Version="8.0.6" Condition=" '$(TargetFramework)' == 'net6.0' " /> | ||
| <PackageReference Include="System.Text.Json" Version="10.0.3" Condition=" '$(TargetFramework)' == 'net6.0' " /> |
There was a problem hiding this comment.
Why this increase/change? Should not be.
| <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="SocketIOClient" Version="3.1.2" /> | ||
| <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="10.0.3" /> |
There was a problem hiding this comment.
Should not be necessary - like the changes using it.
| private bool _isDisposed; | ||
|
|
||
| public SocketIOConnection(string uri, string authorization) | ||
| public SocketIOConnection(string uri, string authorization, Action<IServiceCollection> configureSocketIO = null) |
There was a problem hiding this comment.
Everything that references IServiceCollection should be reverted.
There is no need (and architectural no gain) forwarding the service collection or an action using it.
If we take options (e.g., when configured in a DI) we should be able to resolve this much earlier (in a DI setup) and then forward accordingly.
FlorianRappl
left a comment
FlorianRappl
left a comment
There was a problem hiding this comment.
In general this is fine, but:
- ElectronNET.API should have nothing to do with DI
- It should not be necessary to make as many changes we've seen here
I think taking this a bit more lightweight would benefit the PR.
|
I'll close this for now. The SocketIO dependency-update should not require so many changes and the proxy is already disabled in |
2 participants
Co-Authored-By: Claude Sonnet 5