Skip to content

remove proper noun Ethereum from all locales for onlyAddTrustedNetworks#10598

Merged
brad-decker merged 1 commit intodevelopfrom
remove-ethereum-from-malicious-network-message
Mar 8, 2021
Merged

remove proper noun Ethereum from all locales for onlyAddTrustedNetworks#10598
brad-decker merged 1 commit intodevelopfrom
remove-ethereum-from-malicious-network-message

Conversation

@brad-decker
Copy link
Copy Markdown
Contributor

Fixes: #10388

Explanation:
Removes proper noun 'Ethereum' from the translation strings. Going to need help verifying all locales. Note that I did not remove anything from the ko translation because the word Ethereum did not appear in the string.

@brad-decker brad-decker requested a review from Gudahtt March 5, 2021 16:53
@Gudahtt Gudahtt changed the title remove proper noun Ethereum from all locales for onlyAddTrustedNetwroks remove proper noun Ethereum from all locales for onlyAddTrustedNetworks Mar 5, 2021
@brad-decker brad-decker marked this pull request as ready for review March 5, 2021 17:36
@brad-decker brad-decker requested a review from a team as a code owner March 5, 2021 17:36
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [1aa116e]
Page Load Metrics (793 ± 45 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint58876884
domContentLoaded4689497929545
load4699507939445
domInteractive4679497919545

darkwing
darkwing previously approved these changes Mar 8, 2021
Copy link
Copy Markdown
Contributor

@darkwing darkwing 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
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

We might want to consider removing some or all of these translations. We might be making them confusing/grammatically incorrect by removing this term.

Also some of them still seem to include translations of Ethereum. The ko message still includes 'Ethereum' in the output when translated with Google Translate for example.

@brad-decker
Copy link
Copy Markdown
Contributor Author

We might want to consider removing some or all of these translations. We might be making them confusing/grammatically incorrect by removing this term.

Should we just go the safest route and remove them, waiting for lionbridge to update them in the future? cc @Gudahtt

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Mar 8, 2021

That's essentially what we've done in most other cases where the meaning of the message has changed. Deleting them seems appropriate. If there were any where you felt reasonable confident the sentences still makes sense without 'Ethereum' we could make exceptions though.

@brad-decker brad-decker force-pushed the remove-ethereum-from-malicious-network-message branch from 1aa116e to ebd9df4 Compare March 8, 2021 16:57
@brad-decker
Copy link
Copy Markdown
Contributor Author

In the end I removed a handful of translations that just didn't seem correct or to match the original intent of the message, and kept a few. I removed the Korean translation as it included the word Ethereum in it, so we'll need a new translation for that locale. The same goes for the simplified Chinese.

I removed Japanese and Filipino translations because the google translate seemed off to me and I want to ensure we get accurate translations.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [ebd9df4]
Page Load Metrics (606 ± 29 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45795894
domContentLoaded4537046056029
load4547056066029
domInteractive4527046046029

@brad-decker brad-decker force-pushed the remove-ethereum-from-malicious-network-message branch from ebd9df4 to 9125975 Compare March 8, 2021 18:10
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [9125975]
Page Load Metrics (635 ± 21 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint48776084
domContentLoaded5067006344321
load5077026354421
domInteractive5047006334421

@brad-decker brad-decker merged commit 9f11fad into develop Mar 8, 2021
@brad-decker brad-decker deleted the remove-ethereum-from-malicious-network-message branch March 8, 2021 18:28
@github-actions github-actions bot locked and limited conversation to collaborators Mar 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update custom network notification wording

4 participants