-
Notifications
You must be signed in to change notification settings - Fork 1
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
Hotfix translations. #144
Hotfix translations. #144
Conversation
@@ -142,10 +144,7 @@ const PreflightResults = ({ preflight }: { preflight: Preflight }) => { | |||
return null; | |||
} | |||
|
|||
const { failed, message } = getErrorInfo({ | |||
preflight, | |||
label: i18n.t('Pre-install validation'), |
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.
Because this label was getting double-translated, in non-EN languages i18n was looking for a translation key e.g. Validatie vóór installatie encountered
for the second translation, instead of key Pre-install validation encountered
.
@@ -106,7 +106,7 @@ | |||
"css-loader": "^3", | |||
"eslint": "^7.5.0", | |||
"eslint-config-prettier": "^6.11.0", | |||
"eslint-import-resolver-typescript": "^2.2.0", | |||
"eslint-import-resolver-typescript": "^1.1.1", |
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.
This was only triggering warnings, but since v2.0 this plugin doesn't work well (at least without other changes I haven't dug into). Cf import-js/eslint-import-resolver-typescript#33.
: `${i18n.t('Pre-install validation encountered')} ${msg}.`; | ||
} else { | ||
info.message = `${i18n.t('Installation encountered')} ${msg}.`; | ||
} |
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.
@jgerigmeyer wait... shouldn't this include the word "error" before "encountered"?
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.
No, this isn't "Installation error encountered..." -- it's "Installation encountered [3 errors]"
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.
@jgerigmeyer ah, I see. In that case we should probably be using interpolation so that the translators have the full context, and so that we can translate into languages that have a different order of subject-verb-object.
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.
@davisagli Yes, I think that's true. We're using it a few places, but not consistently -- and I don't think there's a good reason for that.
...also pin back eslint plugin version.
Cute animal pic
Because everyone likes pictures of animals.
See https://oddbird.slack.com/archives/CB2576XT4/p1597877115018500