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

feat: initial work on i18n #66

Closed
wants to merge 1 commit into from
Closed

feat: initial work on i18n #66

wants to merge 1 commit into from

Conversation

molant
Copy link
Contributor

@molant molant commented Jul 9, 2021

No description provided.

@MarshallOfSound MarshallOfSound temporarily deployed to electronjsorg-new-pr-66 July 9, 2021 19:20 Inactive
@ckerr ckerr marked this pull request as ready for review July 10, 2021 17:54
@ckerr ckerr marked this pull request as draft July 10, 2021 17:54
@molant molant force-pushed the i18n branch 3 times, most recently from e5a9a75 to 35da028 Compare July 14, 2021 21:08
@MarshallOfSound MarshallOfSound temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 21:08 Inactive
@MarshallOfSound MarshallOfSound temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 21:11 Inactive
@@ -12,6 +12,10 @@ module.exports = {
favicon: 'img/favicon.ico',
organizationName: 'electron',
projectName: 'electron',
i18n: {
defaultLocale: 'en',
locales: ['en', 'es-ES']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing the Spanish one for now although all the translations are pulled and organized.
We could keep this in sync automatically with Crowdin somehow (during the prebuild step probably?)

@@ -99,7 +107,7 @@ module.exports = {
],
},
],
copyright: `Copyright © ${new Date().getFullYear()} My Project, Inc. Built with Docusaurus.`,
copyright: `Copyright © OpenJS Foundation and Electron contributors.`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the year because the generated JSON had it hardcoded somehow instead of doing interpolation and thus it will be kept out of date.

@MarshallOfSound MarshallOfSound had a problem deploying to electronjsorg-new-pr-66 July 14, 2021 21:16 Failure
@molant molant temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 21:17 Inactive
@MarshallOfSound MarshallOfSound had a problem deploying to electronjsorg-new-pr-66 July 14, 2021 21:18 Failure
@molant molant temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 21:19 Inactive
@@ -4,38 +4,6 @@ const fs = require('fs').promises;
const path = require('path');
const globby = require('globby');

/** The keywords that need to be escaped so MDX does not complain */
const keywords = new Set([
Copy link
Contributor Author

@molant molant Jul 14, 2021

Choose a reason for hiding this comment

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

This wasn't being used at the end so I delete it.

@@ -0,0 +1,136 @@
//@ts-check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be mostly the previous contents of download-docs.js.

@@ -0,0 +1,82 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was docs-reorg.json that I renamed to match reorg-docs.js.

if (build.status === 'finished') {
break;
} else {
console.log(`Crowdin status: Waiting ${interval} seconds (retry ${i}/${counter})`)
Copy link
Contributor Author

@molant molant Jul 14, 2021

Choose a reason for hiding this comment

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

In my tests I've got builds done at around 3.5 minutes (interval 7). We could increate the counter a bit more if we want to be extra cautious.

GITHUB_TOKEN=
CROWDIN_TOKEN=
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to add this tokens to Heroku

@MarshallOfSound MarshallOfSound temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 21:31 Inactive
@MarshallOfSound MarshallOfSound temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 22:06 Inactive
@molant molant temporarily deployed to electronjsorg-new-pr-66 July 14, 2021 22:15 Inactive
@molant molant marked this pull request as ready for review July 14, 2021 22:59
@molant
Copy link
Contributor Author

molant commented Jul 14, 2021

This should be ready for an initial round of feedback.
The biggest blocker at the moment I think it's that the routes are /docs/latest/es-ES. It will probably be better for them to be /es-ES/docs/latest/ and all be lower case.

This will touch docusaurus and CDN configurations. And we might want to consider the scenario for versioned docs as well.

@molant molant requested a review from erickzhao July 14, 2021 23:07
@molant molant marked this pull request as draft July 14, 2021 23:07
@molant molant mentioned this pull request Oct 7, 2021
@molant
Copy link
Contributor Author

molant commented Oct 7, 2021

Closing in favor of #112

@molant molant closed this Oct 7, 2021
@molant molant deleted the i18n branch October 20, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants