Skip to content

codespace: Rename errors from Live Share to generic codespace#4705

Merged
josebalius merged 1 commit intotrunkfrom
jg/rename-errs
Nov 9, 2021
Merged

codespace: Rename errors from Live Share to generic codespace#4705
josebalius merged 1 commit intotrunkfrom
jg/rename-errs

Conversation

@josebalius
Copy link
Contributor

@josebalius josebalius commented Nov 8, 2021

Closes internal issue codespaces#4496

Removes mention of Live Share in error messages shown to users.

@josebalius josebalius self-assigned this Nov 8, 2021
@josebalius josebalius requested a review from a team as a code owner November 8, 2021 13:37
Copy link
Contributor

@adonovan adonovan left a comment

Choose a reason for hiding this comment

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

This seems fine locally, but I suspect it may lead to the construction of errors that are both repetitive ("connecting to codespace: connecting to codespace: ...") and ambiguous: there are multiple control paths that might result in an error with the prefix "connecting to codespace: ...".

Ideally an integration test suite would assert the text of the errors in specific scenarios, causing us to consider and disambiguate them. In the absence of that integration test suite, please be on the lookout for opportunities to improve unclear error messages as they arise.

Copy link

@Lukeghenco Lukeghenco left a comment

Choose a reason for hiding this comment

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

RIP Live Share

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Agreed that Live Share is an implementation detail and as such, the user doesn't have to be aware of it.

Re: repetitive messages in wrapped errors: let's see how this turns out. If the final error messaging proves to be redundant, we can polish that subsequently.

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.

4 participants