Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tolerate snippets where Shortcut is null. #31738

Merged
merged 1 commit into from Feb 23, 2019
Merged

Tolerate snippets where Shortcut is null. #31738

merged 1 commit into from Feb 23, 2019

Conversation

KirillOsenkov
Copy link
Member

Simple null-check for snippet.Shortcut in preprocessor context check.

@KirillOsenkov
Tolerate snippets where Shortcut is null.
96bdaee 
Simple null-check for snippet.Shortcut in preprocessor context check.
@KirillOsenkov KirillOsenkov requested a review from a team as a code owner December 13, 2018 02:51
@vatsalyaagrawal vatsalyaagrawal added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Dec 13, 2018
@vatsalyaagrawal vatsalyaagrawal added this to InQueue in IDE: CommunityPR via automation Dec 13, 2018
@jinujoseph jinujoseph added this to the 16.0 milestone Jan 2, 2019
@jinujoseph jinujoseph moved this from InQueue to BuddyAssigned in IDE: CommunityPR Jan 2, 2019
heejaechang
heejaechang approved these changes Jan 17, 2019
View reviewed changes
Copy link
Contributor

@heejaechang heejaechang left a comment

Choose a reason for hiding this comment

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

thank you Kirill!

@heejaechang
Copy link
Contributor

@jinujoseph it is preview 3, so I need your approval?

@sharwell
Copy link
Member

❓ Is this something we can add a test for?
❓ Is there an issue that led to this that we can link?

@jinujoseph jinujoseph modified the milestones: 16.0, 16.0.P3 Jan 17, 2019
@jinujoseph
Copy link
Contributor

@KirillOsenkov could you pls add more context to the issue

@KirillOsenkov
Copy link
Member Author

This is not a blocking or urgent issue. As far as I could see it is not user visible on Windows. But in other environments (VSMac) some snippets may have Shortcut set to null.

I'm OK with postponing it if you like, I just want to make sure this doesn't get lost long term.

@heejaechang
Copy link
Contributor

@jinujoseph is there any reason this is now blocked and lost? can I check this in?

@jinujoseph jinujoseph modified the milestones: 16.0.P3, 16.1.P1 Feb 7, 2019
@jinujoseph
Copy link
Contributor

@heejaechang , lets retarget 16.1.preview1 and merge or hold till we snap from master

@jinujoseph jinujoseph moved this from BuddyAssigned to LGTM in IDE: CommunityPR Feb 7, 2019
@heejaechang heejaechang merged commit 4239268 into dotnet:master Feb 23, 2019
IDE: CommunityPR automation moved this from LGTM to Completed Feb 23, 2019
@KirillOsenkov KirillOsenkov deleted the FixSnippetPreprocessorNullRef branch February 23, 2019 05:05
@jinujoseph jinujoseph moved this from Completed-149 to Completed-2019 in IDE: CommunityPR Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Therzok Therzok Therzok approved these changes

@heejaechang heejaechang heejaechang approved these changes

Assignees

@heejaechang heejaechang

Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
IDE: CommunityPR
  
Completed-2019
Milestone
16.1.P1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@KirillOsenkov @heejaechang @sharwell @jinujoseph @Therzok @vatsalyaagrawal