-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Conversation
|
||
End Function | ||
|
||
<CLSCompliant(False)> |
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.
You can remove any CLSCompliant attributes. We don't use 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.
Hmm, I get various errors if I do this though:
"C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\Microsoft.VisualBasic.sln" (default target) (1) ->
"C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\tests\Microsoft.VisualBasic.Tests.csproj.metaproj" (d
efault target) (2) ->
"C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj.metaproj" (default t
arget) (3) ->
"C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj" (default target) (5
) ->
(CoreCompile target) ->
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(322,32): error BC40027: Return type of function 'ToSByte' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub\
corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(343,32): error BC40027: Return type of function 'ToSByte' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub\
corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(613,32): error BC40027: Return type of function 'ToUShort' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub
\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(634,32): error BC40027: Return type of function 'ToUShort' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub
\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(899,32): error BC40027: Return type of function 'ToUInteger' is not CLS-compliant. [C:\Users\hughb\Documents\GitH
ub\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(920,32): error BC40027: Return type of function 'ToUInteger' is not CLS-compliant. [C:\Users\hughb\Documents\GitH
ub\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(1189,32): error BC40027: Return type of function 'ToULong' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub
\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(1212,32): error BC40027: Return type of function 'ToULong' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub
\corefx\src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(2201,55): error BC40028: Type of parameter 'Value' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub\corefx\
src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
C:\Users\hughb\Documents\GitHub\corefx\src\Microsoft.VisualBasic\src\Microsoft\VisualBasic\CompilerServices\Conversio
ns.vb(2209,55): error BC40028: Type of parameter 'Value' is not CLS-compliant. [C:\Users\hughb\Documents\GitHub\corefx\
src\Microsoft.VisualBasic\src\Microsoft.VisualBasic.vbproj]
0 Warning(s)
10 Error(s)
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.
Hmm OK fair enough
NumberStyles.AllowTrailingWhite Or | ||
NumberStyles.AllowCurrencySymbol | ||
|
||
|
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.
Nit, there are various double newlines in this file
Return Value.ToString() | ||
|
||
Public Shared Function FromCharArraySubset(ByVal Value() As Char, ByVal StartIndex As Integer, ByVal Length As Integer) As String | ||
' This is a private function used from the debug windows (VS7 264234) |
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.
Please remove references to old bug databases eg (VS7 264234)
.. VS7 = Visual Studio 2002 ... I doubt any of us could find that bug database any more.
{ | ||
Assert.Throws<InvalidOperationException>(() => Conversions.ToChar(value)); | ||
} | ||
|
||
private static object s_floatEnum; | ||
|
||
public static object FloatEnum |
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.
You can't make a float enum in VB or C#. Is it worth bothering to test this scenario? Ref.Emit is nice to avoid.
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.
Same for bool and char. I would just skip 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.
These edge cases used to throw a different exception in .NET Core compared to .NET Framework or threw when netfx didn't, so I would advocate keeping them. No harm really and there are already tests dotted around the place that use IntPtr/float etc. enums
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.
Ok
yield return new object[] { char.MinValue }; | ||
yield return new object[] { (char)1 }; | ||
yield return new object[] { char.MaxValue }; | ||
yield return new object[] { new DateTime(10) }; |
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.
Are these cases no longer relevant?
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.
I think they’re consolidated with the test data for other methods as they’re common with all other conversion methods
Do you want to rebase @hughbe then we can probably merge? |
@dotnet-bot test this please |
@hughbe when I merge this I would like to squash the commits, do you have a problem if I do that on my end? Or would you prefer to manually squash them yourself? |
@333fred please squash it, that's what we do, unless there is strong reason to keep the PR change history. |
+1, in CoreFX and CoreCLR we always "squash and merge" unless there's good reason - I know other repos have different policies. |
Thanks @hughbe, this has been merged. |
Fixes https://github.com/dotnet/corefx/issues/31482