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

Use completion API #583

Merged
merged 6 commits into from
Jun 14, 2016
Merged

Conversation

DustinCampbell
Copy link
Contributor

This change updates the existing AutoComplete end point to use the Roslyn Completion API, which allows the old keyword recommender code to be deleted. In order to properly support the Roslyn Completion API and all of its features, we'll need a new endpoint. I'll be working on that next.

cc @david-driscoll, @troydai

Note: The last commit is huge and is mostly around deleting code. If you want to review the meatier changes, take a look at the earlier commits.

@DustinCampbell
Copy link
Contributor Author

Looks like dotnet restore is failing on CI builds. Does anyone know anything about that? @troydai?

@troydai
Copy link
Contributor

troydai commented Jun 14, 2016

Looking at it

@troydai
Copy link
Contributor

troydai commented Jun 14, 2016

NuGet references are out-of-date. I'm going to update them,.

@DustinCampbell
Copy link
Contributor Author

Yeah, I just cleared my NuGet caches and can reproduce locally.

@troydai
Copy link
Contributor

troydai commented Jun 14, 2016

Sending out PR to address this: #584

You can rebase your changes onto my fix.

@DustinCampbell
Copy link
Contributor Author

Yay! The CI builds have passed. Just need some code reviews.


public static string GetRoslynAssemblyFullName(string name)
{
return $"{name}, Version={RoslynVersion}, Culture=neutral, PublicKeyToken={RoslynPublicKeyToken}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Issue filed: #585

@troydai
Copy link
Contributor

troydai commented Jun 14, 2016

LGTM

@DustinCampbell
Copy link
Contributor Author

Awesome! @david-driscoll, when you get a chance, could you take a look? It's a pretty big change, so I want to be sure you're OK with it.

@david-driscoll
Copy link
Member

:shipit:

@DustinCampbell
Copy link
Contributor Author

Thanks!

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