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

Move DocumentHighlights data types down to the 'Features' layer. #19525

Merged
merged 7 commits into from
May 17, 2017

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented May 15, 2017

No description provided.

@CyrusNajmabadi CyrusNajmabadi added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label May 15, 2017
@CyrusNajmabadi CyrusNajmabadi removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label May 15, 2017
@CyrusNajmabadi
Copy link
Member Author

This is just resending: #18972 out again.

We rolled that back because of some infrastructure problems we were ahving around insertions.

Note: this is the change that changes our API layering and will affect TypeScript and F# as welll. Tagging @paulvanbrenk @minestarks @brettfo

I'm going to try to get this in today and i will let you know when it makes it through and is published so you can pull the latest Roslyn version and update your bits according.

If i'm not mistaken these bits will flow into vsuml. But @Pilchie and @jasonmalinowski can confirm this.

@Pilchie
Copy link
Member

Pilchie commented May 15, 2017

Starting today, we'll be targeting insertions to d15prerel as well as vsuml

@CyrusNajmabadi
Copy link
Member Author

Ok. @tmeschter Is very smart. I changed the API to leave hte old API in and to add the new API. The roslny impl will look for either export and will support both until the point that F# and TypeScript are able to move over.

Once this is checked in, i will mildly poke TypeScript and F# until they are able to move over.

Note: @minestarks As you noticed, we have changed Roslyn a bit (especially some enum values). So no matter what, you should move to a more recent version and recompile to fix things up.

@CyrusNajmabadi
Copy link
Member Author

This is now no longer a breaking change to Roslyn, and so can go in much more gently.

@CyrusNajmabadi
Copy link
Member Author

Telling @MattGertz aobut this. This is just a resend of #18972 which was already approved. The original was rolled because because we didn't want to take the breaking change in it at a time when we were trying to lock down the insertion.

Tom proposed a way to do it again without any breaks, and so that's what we're taking now. This means that we don't need to coordinate any insertion, and Roslyn can unblock itself wrt OOP work here without impacting F# or TypeScript.

@MattGertz
Copy link
Contributor

Approved pending tests.

@CyrusNajmabadi CyrusNajmabadi merged commit 3f35adf into dotnet:master May 17, 2017
@CyrusNajmabadi CyrusNajmabadi deleted the moveDocHighlights branch May 17, 2017 19:20
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.

4 participants