-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
Please provide a screenshot of how this will look. There are also conflicts in your branch. |
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.
LGTM but agree with @mykola-mokhnach on il8n stuff.
…n.js + make sure we provide the necessary strings for translation
Thanks for the reviews! I've updated the pull request. |
Thanks for the additional reviews, I've updated the pull request with the necessary i18n changes. |
const {server, setServerParam, t} = this.props; | ||
|
||
const testingbotKeyPlaceholder = process.env.TB_KEY ? | ||
t('usingDataFoundIn', {environmentVariable: 'TB_KEY'}) : t('yourUsername'); |
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.
Where usingDataFoundIn
and yourUsername
resources are defined?
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.
@mykola-mokhnach they are defined in
"usingDataFoundIn": "Using data found in {{environmentVariable}}", |
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.
"yourUsername": "Your Username", |
@testingbot thanks for the contribution! I noticed that TestingBot is based in Belgium. I don't suppose you have any French speakers who would be interested in helping with localization? |
This commit adds TestingBot as a Cloud Provider.
Please let me know if any changes are required.