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

construct scripting workspace information directly from the ProjectInfo #1272

Merged
merged 7 commits into from
Aug 28, 2018

Conversation

filipw
Copy link
Member

@filipw filipw commented Aug 17, 2018

This is a follow up to #1112 which we shipped but it didn't have a real test 😃

In this PR:

  • I fixed a "bug" where RSP based references/namespaces were not respected by the Workspace information endpoint in scripting (construct that info from ProjectInfo directly, which is most reliable way)
  • better late than never - added an integration test for the RSP based configuration of scripting project system
  • fixed some suspicious test code

Copy link
Member

@david-driscoll david-driscoll left a comment

Choose a reason for hiding this comment

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

:shipit: 👍 💯

{
if (a is PortableExecutableReference portableExecutableReference)
{
return portableExecutableReference.FilePath ?? portableExecutableReference.Display;
Copy link
Member

Choose a reason for hiding this comment

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

Is this ever null?

Copy link
Member Author

Choose a reason for hiding this comment

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

not at the moment, but once we support plugins to meddle with this project system it might 😃

@david-driscoll
Copy link
Member

Updated to ensure that everything is passing with the latest stuff

@filipw
Copy link
Member Author

filipw commented Aug 28, 2018

I added a changelog update

@filipw filipw merged commit 9e76cbf into master Aug 28, 2018
@filipw filipw deleted the feature/csx-rsp-tests branch August 28, 2018 11: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.

2 participants