Skip to content

Expose a public API for building RQNames#2274

Merged
Pilchie merged 2 commits intodotnet:masterfrom
Pilchie:PublicRQNameBuilder
Apr 29, 2015
Merged

Expose a public API for building RQNames#2274
Pilchie merged 2 commits intodotnet:masterfrom
Pilchie:PublicRQNameBuilder

Conversation

@Pilchie
Copy link
Member

@Pilchie Pilchie commented Apr 26, 2015

  • Rename RQNameService to RQNameBuilder and make public.
  • Rename TryBuildForPublicApi to Build and have it return null.
  • Remove TryBuild, since we only build RQNames for public Api's now.
  • Clean up implementation to not pass flag around for buildForPublicApi
  • Remove RQDynamicType
  • Create a singleton for System.Object.
  • Add a comment explaining why we erase dynamic.

Fixes #55.

@Pilchie
Copy link
Member Author

Pilchie commented Apr 26, 2015

@mattwar - are you happy with the API shape?
@dpoeschl - okay with the simplification around buildForPublicAPI

@mattwar
Copy link
Contributor

mattwar commented Apr 26, 2015

The type should be called RQName, and the method should be called Create or From.
It should not appear in a namespace called Implementation.
Is there not also an API that converts back to a symbol?

* Rename RQNameService to RQName and make public.
* Rename TryBuildForPublicApi to From and have it return null.
* Remove TryBuild, since we *only* build RQNames for public Api's now.
* Clean up implementation to not pass flag around for buildForPublicApi
* Remove RQDynamicType
* Create a singleton for System.Object.
* Add a comment explaining why we erase dynamic.

Fixes dotnet#55.
@Pilchie Pilchie force-pushed the PublicRQNameBuilder branch from fd24765 to 4f7cfc3 Compare April 26, 2015 17:55
@Pilchie
Copy link
Member Author

Pilchie commented Apr 26, 2015

Thanks! Renamed Build to From and moved to the root Microsoft.VisualStudio.LanguageServices namespaces.

There is no API that converts back, because we don't ever take RQNames as input - we just give them out when we are doing rename/go to definition.

@Pilchie
Copy link
Member Author

Pilchie commented Apr 26, 2015

Not saying we couldn't/shouldn't add a method to go back to ISymbol, just that we don't have one right now.

@mattwar
Copy link
Contributor

mattwar commented Apr 27, 2015

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong sorting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, fixed

@dpoeschl
Copy link
Contributor

👍

@Pilchie Pilchie force-pushed the PublicRQNameBuilder branch from a191ce5 to a327f9b Compare April 28, 2015 22:52
Pilchie added a commit that referenced this pull request Apr 29, 2015
Expose a public API for building RQNames

* Rename RQNameService to RQName and make public.
* Rename TryBuildForPublicApi to From() and have it return null.
* Remove TryBuild, since we only build RQNames for public Api's now.
* Clean up implementation to not pass flag around for buildForPublicApi
* Remove RQDynamicType
* Create a singleton for System.Object.
* Add a comment explaining why we erase dynamic.

Fixes #55.
@Pilchie Pilchie merged commit 7aaefda into dotnet:master Apr 29, 2015
@Pilchie Pilchie deleted the PublicRQNameBuilder branch May 22, 2015 15:40
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.

Expose RQNameService.TryBuildForPublicAPIs to enable language interop in VS.

4 participants