Unix sockets support #499
Unix sockets support #499
Conversation
| @@ -1,7 +1,7 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
|
|
|||
| <PropertyGroup> | |||
| <TargetFrameworks>netstandard2.0;net35;net472</TargetFrameworks> | |||
| <TargetFramework>net8.0</TargetFramework> | |||
There was a problem hiding this comment.
Before merging, we'll have to restore the compatibility with the older frameworks.
| instance = Activator.CreateInstance(implementingType) | ||
| ?? throw new InvalidOperationException($"Unable to create instance of: {implementingType.ToString(true)}"); |
There was a problem hiding this comment.
(MInor) Sorry, why this change? It is not practical to expect null from Activator.CreateInstance: it will only return null if you pass a Nullable as the implementingType. It will throw an exception by itself otherwise; it doesn't need our additional assertion here.
| var elementType = typeInfo.GetElementType(); | ||
| if (elementType == null) return false; |
There was a problem hiding this comment.
Should never be null here, I'd prefer this code to fail in such a case (as it was before) instead of trying to handle the logic error.
| @@ -68,13 +68,13 @@ internal static FieldInfo[] GetBindableFields(TypeInfo typeInfo) | |||
| return list.ToArray(); | |||
| } | |||
|
|
|||
| private static IEnumerable<FieldInfo> GetFields(Type type, Type baseType) | |||
| private static IEnumerable<FieldInfo> GetFields(Type? type, Type baseType) | |||
There was a problem hiding this comment.
Huh, should type really be nullable here? Who calls this on a null type?
| public Client(Lifetime lifetime, IScheduler scheduler, string path, string? optId = null) : | ||
| this(lifetime, scheduler, EndPointWrapper.CreateUnixEndPoint(path), optId) {} |
There was a problem hiding this comment.
(Minor) Since this an unusual use case, I'd prefer to have something other than a string in here. Let's maybe make a separate struct, like this?
public struct UnixSocketConnectionParams
{
public string Path { get; set; }
}and then
public Client(Lifetime lifetime, IScheduler scheduler, UnixSocketConnectionParams connectionParams, string? optId = null)
WDYT?
| // [SecurityPermission(SecurityAction.Demand, SerializationFormatter = true)] | ||
| [Obsolete("Obsolete")] | ||
| protected RdFault(SerializationInfo info, StreamingContext context) : base(info, context) | ||
| { | ||
| ReasonTypeFqn = info.GetString(nameof(ReasonTypeFqn)); | ||
| ReasonText = info.GetString(nameof(ReasonText)); | ||
| ReasonMessage = info.GetString(nameof(ReasonMessage)); | ||
| } | ||
|
|
||
| [SecurityPermission(SecurityAction.Demand, SerializationFormatter = true)] | ||
| // [SecurityPermission(SecurityAction.Demand, SerializationFormatter = true)] | ||
| [Obsolete("This API supports obsolete formatter-based serialization. It should not be called or extended by application code.", DiagnosticId = "SYSLIB0051", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] |
There was a problem hiding this comment.
A note to not forget to do something about these eventually. I'm not sure we use them at all.
| } | ||
| } | ||
| #endif | ||
| // #if !NET35 |
There was a problem hiding this comment.
Note to uncomment it eventually :)
| <!--suppress MsbuildTargetFrameworkTagInspection --> | ||
| <TargetFrameworks>netcoreapp3.1</TargetFrameworks> | ||
| <TargetFramework>net8.0</TargetFramework> | ||
| <TargetFrameworks Condition=" $(OS) == 'Windows_NT' ">net472;$(TargetFrameworks);net35</TargetFrameworks> |
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net472</TargetFramework> | ||
| <TargetFramework>net8.0</TargetFramework> |
There was a problem hiding this comment.
Don't forget to port this to both platforms somehow.
| @@ -1,6 +0,0 @@ | |||
| { | |||
| "sdk": { | |||
| "version": "6.0.100", | |||
There was a problem hiding this comment.
Let's update it to net8 better :)
No description provided.