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(theme-translations): add extra Korean translation, fix typo #5976

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

revi
Copy link
Contributor

@revi revi commented Nov 20, 2021

Motivation

Updated a few translation string that was left untranslated (probably added after the initial translation?). Those left are because I had no idea where it belongs to. Also fixing a typo with Version.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

This probably does not require test plans.

Related PRs

N/A

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 20, 2021
@netlify
Copy link

netlify bot commented Nov 20, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 436c74a

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61afaa77aca90700075b9581

😎 Browse the preview: https://deploy-preview-5976--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Nov 20, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 97
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5976--docusaurus-2.netlify.app/

@Josh-Cena
Copy link
Collaborator

Hi, thanks for your time on this! #5849 has done some refactorings to the translation system, which will cause merge conflicts in your PR after that one got merged (or vice versa: we merge yours and have to fix the conflicts in that one.) We'd also like to try out the new translation workflow. So while your PR looks good to me (especially the fact that it fixes the "ko only has one plural form" warning), would you kindly stay with us for a while and resolve the conflicts after we merge #5849?

Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Not experienced with Korean at all but hope these comments can help you complete the translations

@revi
Copy link
Contributor Author

revi commented Nov 20, 2021

Hi, thanks for your time on this! #5849 has done some refactorings to the translation system, which will cause merge conflicts in your PR after that one got merged (or vice versa: we merge yours and have to fix the conflicts in that one.) We'd also like to try out the new translation workflow. So while your PR looks good to me (especially the fact that it fixes the "ko only has one plural form" warning), would you kindly stay with us for a while and resolve the conflicts after we merge #5849?

This one can wait. Just ping me when the 5849 is merged and this can be worked on.

@Josh-Cena Josh-Cena added pr: polish This PR adds a very minor behavior improvement that users will enjoy. status: blocked This issue is blocked by another issue or external dep and can't be pushed further. labels Nov 20, 2021
Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Thanks 👍

"Blog archive" is not easy to translate. I went for "blog history" in zh-Hans, but "blog list" also sounds good

@slorber
Copy link
Collaborator

slorber commented Nov 20, 2021

Thanks

Theme changes have been merged, it may be simpler to create another new PR with these changes ported to the new system:

https://github.com/facebook/docusaurus/tree/main/packages/docusaurus-theme-translations/locales/ko

@Josh-Cena Josh-Cena removed the status: blocked This issue is blocked by another issue or external dep and can't be pushed further. label Nov 21, 2021
@Josh-Cena
Copy link
Collaborator

At least I wouldn't like seeing a PR closed on my timeline😄 @revi Whether to submit a new one or just resolve conflicts in the current one (or let me do it, if you wish), your call

@revi
Copy link
Contributor Author

revi commented Nov 24, 2021

@Josh-Cena You can feel free to rebase and merge :-) (Thanks in advance!)

@Josh-Cena
Copy link
Collaborator

Ah, okay, I'll do it then. Thanks again!

@Josh-Cena
Copy link
Collaborator

@revi actually, I do not have write access because your fork is an org fork not a personal fork. Do you see a checkbox on the right sidebar to allow write access to maintainers?

@Josh-Cena Josh-Cena changed the title Add extra Korean translation, fix typo feat(theme-translations): add extra Korean translation, fix typo Nov 24, 2021
Signed-off-by: Yongmin Hong <revi@pobox.com>
Resolves most comments, more fix.

Signed-off-by: Yongmin Hong <revi@pobox.com>
add the missing bit.

Signed-off-by: Yongmin Hong <revi@pobox.com>
@Josh-Cena Josh-Cena merged commit 813300b into facebook:main Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants