Skip to content

Simplify construction of hte RemoteWorkspace#63112

Merged
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:simplifyRemote
Aug 2, 2022
Merged

Simplify construction of hte RemoteWorkspace#63112
CyrusNajmabadi merged 2 commits intodotnet:mainfrom
CyrusNajmabadi:simplifyRemote

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner August 1, 2022 19:19
@ghost ghost added the Area-IDE label Aug 1, 2022
internal RemoteWorkspace(HostServices hostServices, string? workspaceKind)
: base(hostServices, workspaceKind)
internal RemoteWorkspace(HostServices hostServices)
: base(hostServices, WorkspaceKind.RemoteWorkspace)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

WorkspaceKind

Is the same RemoteWorkspace instance gonna be used for all remote workspaces? Now that we have WorkspaceKind in Solution snapshot would it be different from Solution.Workspace.Kind for RemoteWorkspace?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ideally, RemoteWorkspace will go away. Since we don't want anything on OOP to even be able to go from solution->Workspace anymore, and there's no way to mutate things in OOP.

This is one of those PRs in the "help cyrus poke around this space to learn what is going on and what unknown unknowns exist" space.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would we return null from Solution.Workspace then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that, or just throwing. i'm experimenting with that in another branch right now. we would at least 'ban' it. :)

@CyrusNajmabadi CyrusNajmabadi merged commit f6f1c71 into dotnet:main Aug 2, 2022
@ghost ghost added this to the Next milestone Aug 2, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the simplifyRemote branch August 2, 2022 06:16
@dibarbet dibarbet modified the milestones: Next, 17.4 P2 Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants