Skip to content

TryParse API for language version no longer an extension method#27461

Merged
jcouv merged 4 commits intodotnet:masterfrom
jcouv:hide-langversion
Jun 11, 2018
Merged

TryParse API for language version no longer an extension method#27461
jcouv merged 4 commits intodotnet:masterfrom
jcouv:hide-langversion

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Jun 5, 2018

Customer scenario

If you reference the compiler nuget package, an extension on the string type becomes available. That has been reported as being annoying (it adds an unexpected completion).
I'm making this API a regular (non-extension) method to remedy this.

Bugs this fixes

Fixes #26132

Workarounds, if any

Risk

Performance impact

Low. Just make the the API non-extension.

@jcouv jcouv added this to the 15.8 milestone Jun 5, 2018
@jcouv jcouv self-assigned this Jun 5, 2018
@jcouv jcouv requested a review from a team as a code owner June 5, 2018 01:05
@etbyrd
Copy link
Copy Markdown
Contributor

etbyrd commented Jun 5, 2018

@dotnet-bot test windows_debug_vs-integration_prtest

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 7, 2018

@dotnet/roslyn-compiler One-liner for review. Thanks

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 7, 2018

From discussion with @cston, I'll ping compat council to see if we can just take the break (not make it an extension method).

@@ -1,5 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.ComponentModel;
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.

using is no longer needed.

Copy link
Copy Markdown
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

:shipit:

@jcouv jcouv changed the title Make TryParse API for language version non-browsable TryParse API for language version no longer an extension method Jun 8, 2018
@jcouv jcouv requested a review from a team as a code owner June 8, 2018 17:43
@jcouv jcouv force-pushed the hide-langversion branch from f5d437e to 96fa301 Compare June 8, 2018 20:00
@jcouv jcouv closed this Jun 8, 2018
@jcouv jcouv reopened this Jun 8, 2018
@jcouv jcouv force-pushed the hide-langversion branch 2 times, most recently from 734fb8d to 96fa301 Compare June 9, 2018 00:36
@jcouv jcouv added the Area-IDE label Jun 9, 2018
@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 11, 2018

@dotnet/roslyn-ide @jinujoseph for a tiny review. I had to update a few lines of IDE code/tests to adjust to this change to a compiler API. Thanks

1 similar comment
@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 11, 2018

@dotnet/roslyn-ide @jinujoseph for a tiny review. I had to update a few lines of IDE code/tests to adjust to this change to a compiler API. Thanks

@jasonmalinowski
Copy link
Copy Markdown
Member

@jcouv PR descripition no longer reflects the content? I see comments about EditorBrowsable.Never but don't see that in the code.

I presume this has gotten whatever breaking change signoffs you need?

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 11, 2018

@jasonmalinowski Thanks. Updated the description.
Yes, the compat council approved this breaking change.

@jinujoseph for ask-mode approval for 15.8. Thanks

@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge from IDE side for 15.8.Preview4

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jun 11, 2018

Thanks
I'm approving from compiler side. FYI @jaredpar

@jcouv jcouv merged commit 1d3ff23 into dotnet:master Jun 11, 2018
@jcouv jcouv deleted the hide-langversion branch June 11, 2018 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LanguageVersionFacts adds a TryParse() extension to the String class

7 participants