-
Notifications
You must be signed in to change notification settings - Fork 182
Make ApiPort to be Global tool. More version updates #790
Conversation
…NET standard1.3 support and only support .NET standard2.0; updated ApiPort console apps to .NET Core 2.1 standalone app and removed desktop app support; update VS extensions to target .net4.6.1. Also fixed some warnings to call some APIs with specific cultureInfo.
…PROXY still is not supported on netcore2.1
@twsouthwick Do you have any additional feedback? |
@@ -106,7 +106,7 @@ public override int GetHashCode() | |||
{ | |||
if (!_hashComputed) | |||
{ | |||
_hashCode = ((DefinedInAssemblyIdentity?.ToString() ?? string.Empty) + (MemberDocId ?? string.Empty) + IsPrimitive.ToString()).GetHashCode() ^ CallingAssembly.GetHashCode(); | |||
_hashCode = ((DefinedInAssemblyIdentity?.ToString() ?? string.Empty) + (MemberDocId ?? string.Empty) + IsPrimitive.ToString((IFormatProvider)CultureInfo.CurrentUICulture)).GetHashCode() ^ CallingAssembly.GetHashCode(); |
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.
Not a blocker, but I don't think CurrentUICulture is appropriate for a hashcode. We should use Invariant here. As a point of preference, this seems like a somewhat iffy way to construct a hashcode. I think we have some hashcode helpers now. I'll see if I can find them.
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.
Ah. Only in Core/ns2.1: https://docs.microsoft.com/en-us/dotnet/api/system.hashcode.combine?view=netstandard-2.1
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.
Use InvariantCulture
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.
fixed
@@ -106,7 +106,7 @@ public override int GetHashCode() | |||
{ | |||
if (!_hashComputed) | |||
{ | |||
_hashCode = ((DefinedInAssemblyIdentity?.ToString() ?? string.Empty) + (MemberDocId ?? string.Empty) + IsPrimitive.ToString()).GetHashCode() ^ CallingAssembly.GetHashCode(); | |||
_hashCode = ((DefinedInAssemblyIdentity?.ToString() ?? string.Empty) + (MemberDocId ?? string.Empty) + IsPrimitive.ToString((IFormatProvider)CultureInfo.CurrentUICulture)).GetHashCode() ^ CallingAssembly.GetHashCode(); |
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.
Use InvariantCulture
Test failures due to missing ilasm is an known SDK issue and fixed in SDK 3.0. more details at https://github.com/dotnet/cli/issues/12341. we can ignore the failures for now. |
This PR includes the following changes:
Make ApiPort to be Global tool (issue #668).
Updated ApiPort console apps to .NET Core 2.1 standalone app and removed desktop app support;
Updated ApiPort libraries to remove NET standard1.3 support and only support .NET standard2.0;
Update VS extensions to target .net4.6.1 to support .NET standard.
Fixed some warnings to call some APIs with specific cultureInfo.