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

core: fix icu error thrown while throwing protocol error #9935

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

brendankenny
Copy link
Member

fixes #9882

We could probably revisit how these protocol errors are generated because this function predates all of the i18n work and most of LHError handling, but this is the easy fix for now.

All of the LHErrors that have patterns to match use the same message, which doesn't have any replacements, so these arguments can safely be removed.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

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

LGTM.

Although it seems like this isn't guaranteed to be true in the future? Like if one did add a ICU replacement. So might be dragons here?

@brendankenny
Copy link
Member Author

brendankenny commented Nov 6, 2019

Although it seems like this isn't guaranteed to be true in the future? Like if one did add a ICU replacement. So might be dragons here?

yeah, but it also isn't guaranteed that these would be the right replacements for those future strings, either :)

I think we'd want to revisit the fromProtocolMessage method overall at that point.

@brendankenny brendankenny changed the title core: fix icu error thrown when throwing protocol error core: fix icu error thrown while throwing protocol error Nov 6, 2019
@brendankenny brendankenny merged commit d53284c into master Nov 6, 2019
@brendankenny brendankenny deleted the protocolicu branch November 6, 2019 21:33
connorjclark added a commit that referenced this pull request Nov 7, 2019
Squashed commit of the following:

commit 1a053c4
Author: Connor Clark <[email protected]>
Date:   Wed Nov 6 17:31:14 2019 -0800

    bundling for devtools

commit 082715b
Author: Brendan Kenny <[email protected]>
Date:   Wed Nov 6 18:41:52 2019 -0500

    feedback

commit 948ee09
Author: Brendan Kenny <[email protected]>
Date:   Tue Nov 5 17:50:55 2019 -0500

    WIP: include plugin in dt build

commit c2a4696
Author: Patrick Hulce <[email protected]>
Date:   Wed Nov 6 15:49:28 2019 -0600

    core: delete full config (#9930)

commit d53284c
Author: Brendan Kenny <[email protected]>
Date:   Wed Nov 6 16:33:41 2019 -0500

    core: fix icu error thrown while throwing protocol error (#9935)
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.

Provided value "protocolMethod" does not match any placeholder in ICU message
5 participants