Skip to content

Follow-up to #42323#42713

Merged
3 commits merged intodotnet:masterfrom
mavasani:FollowUp
Mar 24, 2020
Merged

Follow-up to #42323#42713
3 commits merged intodotnet:masterfrom
mavasani:FollowUp

Conversation

@mavasani
Copy link
Contributor

Address pending PR feedback from #42323

Verified that the first commit resolves the binary breaking change for TS. This commit renames the completion options used by TS to have the 2 suffix for internal use in Roslyn and restores the original option with public type for consumption by TS. Once TS moves to external access model, we can move the field with public type to TS layer, similar to the approach taken for all F# options.

@mavasani mavasani added this to the 16.6 milestone Mar 24, 2020
@mavasani mavasani requested review from a team, jasonmalinowski and sharwell March 24, 2020 04:43
@mavasani mavasani requested a review from a team as a code owner March 24, 2020 04:43
public static readonly PerLanguageOption2<bool> TriggerOnTyping = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTyping), defaultValue: true);

public static readonly PerLanguageOption2<bool> TriggerOnTypingLetters = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTypingLetters), defaultValue: true,
public static readonly PerLanguageOption2<bool> TriggerOnTypingLetters2 = new PerLanguageOption2<bool>(nameof(CompletionOptions), nameof(TriggerOnTypingLetters), defaultValue: true,
Copy link
Contributor Author

@mavasani mavasani Mar 24, 2020

Choose a reason for hiding this comment

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

Ensured that the name argument passed to the option constructor was not changed.

/// explicitly defines all the members from "IOption" type as "IOption" is not available in CodeStyle layer.
/// This ensures that all the sub-types of <see cref="IOption2"/> in either layer see an identical
/// set of interface members.
/// </summary>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this is really helpful.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit d2e50fd into dotnet:master Mar 24, 2020
@mavasani mavasani deleted the FollowUp branch March 24, 2020 11:57
This pull request was closed.
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.

2 participants