Skip to content

Conversation

@KevinRansom
Copy link
Contributor

@KevinRansom KevinRansom commented Jun 7, 2018

Whilst trying to get the tests passing I needed to initialize the UIThread helper class for testing.

It turns out we have two, one for LanguageService base, and One for ProjectSystem.Base. It looks like there was a copy and paste job, because both were in the Blah.LanguageService namespace. I renamed the one in the ProjectSystem library to be in the Blah.ProjectSystem namespace.

Further it turns out that only one of the two helpers were ever initialized on package load, so I now initialize both.

I guess the notion for two seperate assemblies, was to not have the ProjectSystem.base take a dependency on the LanguageService.Base assembly. I guess for this one type that was probably an okay call.

Kevin

@smoothdeveloper
Copy link
Contributor

If this means one step closer to the vs integration tests being enabled again, this is good 🙂

@KevinRansom
Copy link
Contributor Author

@smoothdeveloper I wish I had never noticed they were broken.

@smoothdeveloper
Copy link
Contributor

that doesn't solve the issue that @vasily-kirichenko would have found out 😄

Is there opportunity at later stage to just have one class or they are meant to differ for some reasons in the future?

Not taking the dependency feels clean but not if we had to duplicate large class in both projects.

@KevinRansom
Copy link
Contributor Author

@smoothdeveloper Don moved files about, so the history only goes back to 2016. So I have no idea why there are two of them. However, the ProjectSystem.dll is legacy, and will hopefully go away eventually.

@KevinRansom KevinRansom merged commit 91f4906 into dotnet:dev15.8 Jun 7, 2018
dsyme pushed a commit that referenced this pull request Jun 8, 2018
@KevinRansom KevinRansom deleted the FixUiThread branch July 3, 2018 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants