-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
bugfix/12709 Added localization for some phrases #12710
Conversation
How can I restart the testing? I think, this is not my mistake, it should fix by itself. |
@vince-fugnitto @msujew Could you, please, look at the request and check it?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late feedback.
I'm not sure we should actually localize the error messages - especially since they have quite a lot of technical information that is useless to the actual user (such as a sessionId
or requestId
). I would much rather like to see a generic error message related to the failing debug connection rather than just printing the error as it comes from the debug connection.
Okay, what text should I use in this situation? I just added localization and didn’t think about the meaning then) |
In case any errors related to the connection appears, the frontend should probably just print "Debug connection unexpectedly closed". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not quite what I meant. We should keep the original errors (including original error messages), but print a generic message (i.e. "Debug connection unexpectedly closed") where we catch those errors. That way we still have the option to log the underlying error reason, while still giving the user a reasonable (and translatable) message.
@Zebsterpasha Do you intend to continue with this change? I would close the PR in a few days if there's no further comment. |
@msujew Unfortunately, I think we should close this. |
What it does
Closes #12709
How to test
Just check the keys and add localized phrases)
Review checklist
Reminder for reviewers