Skip to content

Remove duplicated implementations of C# event hookup#49092

Merged
jasonmalinowski merged 1 commit intodotnet:masterfrom
jasonmalinowski:clean-up-duplicate-eventhookup
Nov 9, 2020
Merged

Remove duplicated implementations of C# event hookup#49092
jasonmalinowski merged 1 commit intodotnet:masterfrom
jasonmalinowski:clean-up-duplicate-eventhookup

Conversation

@jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Oct 31, 2020

This feature originally existed in the C# editor layer, but then moved to the VS layer once we split out EditorFeatures.Wpf; we didn't have a CSharpEditorFeatures.Wpf so we had no place to put this since it had dependencies on WPF at the time. It no longer has a direct WPF dependency, so we can move this back to EditorFeatures and simply delete the Cocoa copy.

The copy in EditorFeatures.Cocoa was verified to have no improvements against the current code; there were some differences but those were places where we made later fixes that hadn't been reflected into EditorFeatures.Cocoa.

This will not be merged until @tmat and @davidwengier confirm we're good to take it.

Fixes #34190

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner October 31, 2020 01:17
@jasonmalinowski jasonmalinowski self-assigned this Nov 2, 2020
@jasonmalinowski jasonmalinowski force-pushed the clean-up-duplicate-eventhookup branch from be25f3d to 12fda77 Compare November 2, 2020 19:03
@jasonmalinowski jasonmalinowski requested a review from a team November 2, 2020 19:03
@jasonmalinowski jasonmalinowski force-pushed the clean-up-duplicate-eventhookup branch from 12fda77 to f99e0af Compare November 3, 2020 18:44
Copy link
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

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

The first insertion into VS for Mac using EF.Cocoa from our repo is merged so we're good to start fixing issues.

@davidwengier
Copy link
Member

FYI @DavidKarlas @Therzok

@jasonmalinowski jasonmalinowski force-pushed the clean-up-duplicate-eventhookup branch from f99e0af to df27c68 Compare November 5, 2020 19:53
This feature originally existed in the C# editor layer, but then moved
to the VS layer once we split out EditorFeatures.Wpf; we didn't have a
CSharpEditorFeatures.Wpf so we had no place to put this since it had
dependencies on WPF at the time. It no longer has a direct WPF
dependency, so we can move this back to EditorFeatures and simply delete
the Cocoa copy.

The copy in EditorFeatures.Cocoa was verified to have no improvements
against the current code; there were some differences but those were
places where we made later fixes that hadn't been reflected into
EditorFeatures.Cocoa.

Fixes dotnet#34190
@jasonmalinowski jasonmalinowski force-pushed the clean-up-duplicate-eventhookup branch from df27c68 to bdf8701 Compare November 5, 2020 21:44
@jasonmalinowski jasonmalinowski merged commit 3d60b75 into dotnet:master Nov 9, 2020
@jasonmalinowski jasonmalinowski deleted the clean-up-duplicate-eventhookup branch November 9, 2020 20:00
@ghost ghost added this to the Next milestone Nov 9, 2020
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move EventHookup to EditorFeatures layer

5 participants