Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Expose/test Span-based Convert methods #24474

Merged
merged 1 commit into from
Oct 18, 2017

Conversation

stephentoub
Copy link
Member

@@ -191,6 +191,7 @@ public static partial class Convert
public static int ToBase64CharArray(byte[] inArray, int offsetIn, int length, char[] outArray, int offsetOut, Base64FormattingOptions options) { throw null; }
public static string ToBase64String(byte[] inArray, Base64FormattingOptions options) { throw null; }
public static string ToBase64String(byte[] inArray, int offset, int length, Base64FormattingOptions options) { throw null; }
public static string ToBase64String(System.ReadOnlySpan<byte> bytes, Base64FormattingOptions options = Base64FormattingOptions.None) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this done in corert as well? If not then it will fail on uap and uapaot builds.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert.cs is in the shared partition. Once it's merged into coreclr, it'll be mirrored over to corert.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that's great! I missed that part on your coreclr PR :)

@@ -38,6 +38,7 @@
<ItemGroup Condition="'$(TargetGroup)'!='netstandard'">
<Compile Include="System\BitConverterSpan.cs" />
<Compile Include="System\BitConverter.netcoreapp.cs" />
<Compile Include="System\Convert.netcoreapp.cs" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: This will also run on UWP but I guess we don't have a good naming conventions for non-netstandard stuff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple comments but this looks good once your coreclr PR is merged and CI passes. Thanks for doing it Steve!

@stephentoub
Copy link
Member Author

@dotnet-bot test this please

@stephentoub
Copy link
Member Author

@dotnet-bot test this please

@stephentoub stephentoub merged commit 602d2ed into dotnet:master Oct 18, 2017
@stephentoub stephentoub deleted the convertspan branch October 18, 2017 11:09
@karelz karelz added this to the 2.1.0 milestone Oct 28, 2017
pjanotti pushed a commit to pjanotti/corefx that referenced this pull request Oct 31, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants