Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Implement generic interfaces on Regex collections for netcoreapp1.1#12596

Merged
Priya91 merged 4 commits intodotnet:masterfrom
justinvp:regex_collections
Oct 13, 2016
Merged

Implement generic interfaces on Regex collections for netcoreapp1.1#12596
Priya91 merged 4 commits intodotnet:masterfrom
justinvp:regex_collections

Conversation

@justinvp
Copy link
Copy Markdown
Contributor

Port #1756 from Future to Master and expose the new APIs for netcoreapp1.1.

Part of #2869

@justinvp justinvp force-pushed the regex_collections branch 2 times, most recently from 248c3ee to 4325d7f Compare October 12, 2016 19:48
It conflicts with the actual `System.Text.RegularExpressions.Capture`
type.
Implement IList<T>, IReadOnlyList<T>, and IList on the Regex
collections.
Expose the new APIs in netcoreapp1.1.
<OutputType>Library</OutputType>
<NuGetTargetMoniker>.NETStandard,Version=v1.7</NuGetTargetMoniker>
<DefineConstants Condition="'$(TargetGroup)' == 'netcoreapp1.1'">$(DefineConstants);netcoreapp11</DefineConstants>
</PropertyGroup>
Copy link
Copy Markdown
Contributor

@Priya91 Priya91 Oct 13, 2016

Choose a reason for hiding this comment

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

Instead of defining constant, can you add separate file with partial class that's included only for netcoreapp1.1, that way it's more readable, given the number of additions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was just following the same convention used by all these other netcoreapp1.1 PRs, which define this constant instead of adding new partial ref .cs file:

Let me know if you still prefer the separate file here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes please, the other additions are less lines of code so #if statements are fine. That's not the case here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Contributor

@Priya91 Priya91 left a comment

Choose a reason for hiding this comment

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

LGTM with comments.

@justinvp
Copy link
Copy Markdown
Contributor Author

Thanks for the review, @Priya91.

@Priya91 Priya91 merged commit 6403321 into dotnet:master Oct 13, 2016
@justinvp justinvp deleted the regex_collections branch October 13, 2016 18:55
@karelz karelz modified the milestone: 1.2.0 Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Implement generic interfaces on Regex collections for netcoreapp1.1

Commit migrated from dotnet/corefx@6403321
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