Integrate main i18n tool into build pipeline #22254
Conversation
LeanidShutau
added
Team:Core
💚 Build Succeeded |
|
Ugh, old test results disappeared, will retest to get new ones and check the output out. |
|
retest |
| @@ -33,7 +33,7 @@ window.onload = function () { | |||
| err.style['text-align'] = 'center'; | |||
| err.style['background'] = '#F44336'; | |||
| err.style['padding'] = '25px'; | |||
| err.innerText = '{{i18n 'UI-WELCOME_ERROR' '{"defaultMessage": "Kibana did not load properly. Check the server output for more information."}'}}'; | |||
| err.innerText = '{{i18n 'common.ui.welcomeError' '{"defaultMessage": "Kibana did not load properly. Check the server output for more information."}'}}'; | |||
There was a problem hiding this comment.
nit: please update ids for all existing UI-WELCOME_ messages we have in Kibana (in knb-plugin-helpers tests) and remove src/ui/translations entirely for now.
tasks/verify_translations.js
Outdated
| export default function (grunt) { | ||
| grunt.registerTask('verifyTranslations', async function () { | ||
| module.exports = function (grunt) { | ||
| grunt.registerTask('verifyTranslations', function () { |
There was a problem hiding this comment.
issue: hmm, can you please make run:i18nCheck task exactly like run:typeCheck (define in config/run.js, run it right after 'run:typeCheck' in jenkins:unit and test grunt tasks, with !grunt.option('quick') for the latter task)?
And let's rename scripts/extract_default_translations.js to scripts/i18n_check for now and later on we'll figure out whether to keep this name for extraction purpose or not.
tasks/verify_translations.js
Outdated
| grunt.log.writeln(result); | ||
| resolve(); | ||
| }); | ||
| }).then(done, done); |
There was a problem hiding this comment.
It is likely more obsolete, I would use:
.then(done)
.catch(done)
💔 Build Failed |
|
retest |
💚 Build Succeeded |
There was a problem hiding this comment.
Looks good!
- I still see one usage of
verifyTranslationsintasks/test.js, please remove it and make sure we don't have any other leftovers - While testing I've noticed that we misspelled message here: , it should be
kibana/src/dev/i18n/extract_react_messages.js
Line 115 in d787b64
Empty default message .... is not allowed ....instead ofDefault message ... is not allowed ...
@LeanidShutau, also let's wait for @spalger feedback before merging this.
Hey @spalger, could you please take a brief look at the PR and let us know if it looks ok to you? We're going to integrate Listr and other bells and whistles in the follow up, just need a bare minimum right now :)
💚 Build Succeeded |
src/dev/run_i18n_check.js
Outdated
| outputFormat, | ||
| }); | ||
| } catch (e) { | ||
| console.error(`${chalk.white.bgRed(' I18N ERROR ')} ${e.message || e}`); |
There was a problem hiding this comment.
The run() function implements standardized failure handling, please don't override it as removing the stacktrace from unexpected errors can make them really hard to debug. If you have specific errors that don't require a stack trace please throw a FailError instead, like so:
import { createFailError } from '../run';
// ...
throw createFailError(`${chalk.white.bgRed(' I18N ERROR ')} some error message`);
run() will only print the error message from FailErrors and will print the stacktrace for other unexpected errors.
LeanidShutau
force-pushed
the
feature/integrate-i18n-tools
branch
from
bf7170a to
08f91bc
Compare
💚 Build Succeeded |
|
Thanks @LeanidShutau! |
* Integrate main i18n tool to build process * Resolve comments * Remove old task * Replace default Error with FailError
|
6.x/6.5: b705fed |
Closes #20291