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

CONSOLE-2475: Check in Japanese and Chinese translations #7224

Merged

Conversation

rebeccaalpert
Copy link
Contributor

@rebeccaalpert rebeccaalpert commented Nov 16, 2020

Adding first human translations for Japanese and Chinese from Terry's team.

There are only 123 JSON files in the first commit rather than 124 because the Japanese badge translations didn't differ from the prior machine translation. I confirmed the text matched.

The first commit reflects what was internationalized and present in master on November 4. The second commit is the Chinese translations for the remainder of Sprint 192. The third commit is the remaining Japanese translations for Sprint 192. Commit 4 is the revised translations in response to QE.

Fixes https://issues.redhat.com/browse/CONSOLE-2475.

@openshift-ci-robot openshift-ci-robot added the component/core Related to console core functionality label Nov 16, 2020
@openshift-ci-robot openshift-ci-robot added component/dev-console Related to dev-console component/knative Related to knative-plugin component/olm Related to OLM component/shared Related to console-shared labels Nov 16, 2020
@rebeccaalpert rebeccaalpert added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2020
@rebeccaalpert
Copy link
Contributor Author

Blocked by #7187; need to add zh to language arrays/objects.

@rebeccaalpert
Copy link
Contributor Author

Added Chinese to language selector and webpack config.

@rebeccaalpert rebeccaalpert removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2020
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold for approvals
/assign @yapei @ahardin-rh @sferich888

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2020
@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 17, 2020
Adding first human translations for Japanese and Chinese from Terry's team.

Fixes https://issues.redhat.com/browse/CONSOLE-2475
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2020
@rebeccaalpert
Copy link
Contributor Author

Rebased frontend/webpack.config.ts. Lost the lgtm.

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2020
@ahardin-rh
Copy link

/label docs-approved

@openshift-ci-robot openshift-ci-robot added the docs-approved Signifies that Docs has signed off on this PR label Nov 18, 2020
@openshift-ci-robot openshift-ci-robot added the px-approved Signifies that Product Support has signed off on this PR label Dec 2, 2020
@spadgett
Copy link
Member

spadgett commented Dec 2, 2020

ah ok I think I overlooked that email and was still waiting for a reply. While I think it's ok to just send the deltas, but if there are things that needs to be changed globally in keeping consistency across the UI, then having the entire set of files at early stage will be helpful. I think when we arrived to a stable stage for example maintenance phase, we can start receiving deltas-only. We have implemented some changes to the last set of files you sent me, do you want me to send those to you, or just wait for next drop to happen and we'll have the memory applied to them?

Cheers,
Terry

Hey, @rh-tchuang. If you have the changes ready, it would be great if you could send them. We aren't able to merge the PR until it has QE approval. I worry if we include a lot of new content each time QE will find additional things, and it will be hard to ever get the translations merged. Thanks!

@sferich888 sferich888 removed their assignment Dec 2, 2020
@rh-tchuang
Copy link

rh-tchuang commented Dec 2, 2020 via email

@rh-tchuang
Copy link

Hey Sam,
Understood about the PR approval, however there are specific strings that we need to address within frontend/packages/console-app/locales/ja/tour.json but it wasn't in the second batch for us to fix it as it's not treated as delta. Is there a way for us to fix it? Or perhaps during next round?

Cheers,
Terry

@rebeccaalpert
Copy link
Contributor Author

For anyone following along at home, the required file been sent over and process adjusted off of GitHub.

@yapei - I uploaded the latest set of files in the newest commit. I went through and resolved the comments where it looked like the translation team had made updates. We're waiting on an additional Japanese file. I'll add that once we have it. Let me know if you have additional feedback in the meantime.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2020
@spadgett
Copy link
Member

spadgett commented Dec 3, 2020

Hey Sam,
Understood about the PR approval, however there are specific strings that we need to address within frontend/packages/console-app/locales/ja/tour.json but it wasn't in the second batch for us to fix it as it's not treated as delta. Is there a way for us to fix it? Or perhaps during next round?

Cheers,
Terry

Hey, @rh-tchuang. Is this because we sent only the delta last time? We're at the end of the sprint, so we can go ahead and send a full drop and update it in this PR. We'll send all strings going forward instead of only the delta. Thanks!

@rebeccaalpert
Copy link
Contributor Author

rebeccaalpert commented Dec 3, 2020

Hey @rh-tchuang - I uploaded the revised files. Thanks for those. I went through @yapei's feedback and resolved the comments that were addressed. There are some things that weren't addressed. Would you guys be able to comment on the ones that weren't updated so we can get this further through QE?

It's ok if there were decisions not to translate certain things, but it would be great to have that documented in the comments on GitHub for Ya Dan.

@rh-tchuang
Copy link

Hey, @rh-tchuang. Is this because we sent only the delta last time? We're at the end of the sprint, so we can go ahead and send a full drop and update it in this PR. We'll send all strings going forward instead of only the delta. Thanks!

Hey Sam,
Yeah I think last time there were things that we couldn't address or made consistent because some of those files were treated as completed so not dropped into the second batch for sprint 192, hence our translator reported not able to address the comment made. It's just easier to make global changes when a comment is made against the same string several times. But I'm open to suggestions and if it's going to be problematic for QE, I'm happy to continue receiving deltas - but the same issue may happen again where we found a string that requires to be updated, we updated them in the current drop but not able to change the rest in other files within previous drop.

@rh-tchuang
Copy link

Hey @rh-tchuang - I uploaded the revised files. Thanks for those. I went through @yapei's feedback and resolved the comments that were addressed. There are some things that weren't addressed. Would you guys be able to comment on the ones that weren't updated so we can get this further through QE?

It's ok if there were decisions not to translate certain things, but it would be great to have that documented in the comments on GitHub for Ya Dan.

Hey Rebecca, yeah I've commented on all of them and tagged translator to clarify whether a particular string needs to be localized or not. I will resend you all the files that we have addressed but didn't seem to be picked up. As for some of the files not included within the current drop, we will need that to be sent to us again, and may be we can get a full batch next round so that all identical strings in question can be updated in one go.

@yapei
Copy link
Contributor

yapei commented Dec 4, 2020

@yanpzhan Could you review the comments?

@rebeccaalpert
Copy link
Contributor Author

rebeccaalpert commented Dec 4, 2020

Sorry @yapei! I meant to tag @yanpzhan instead! Won't happen again.

@rebeccaalpert
Copy link
Contributor Author

Hey Rebecca, yeah I've commented on all of them and tagged translator to clarify whether a particular string needs to be localized or not. I will resend you all the files that we have addressed but didn't seem to be picked up. As for some of the files not included within the current drop, we will need that to be sent to us again, and may be we can get a full batch next round so that all identical strings in question can be updated in one go.

Hey @rh-tchuang - Thanks so much! We'll send full file sets rather than deltas going forward. I'll send a batch over today that will include Sprint 193.

@yanpzhan
Copy link
Contributor

yanpzhan commented Dec 7, 2020

@rebeccaalpert @rh-tchuang I reviewed all comments, and those updated items are good to me now, no new issue is found.
Should I add qe-approve label now or wait another review after those addressed in the translation files but not picked in for now in @rh-tfu 's comments to come in?

@rebeccaalpert
Copy link
Contributor Author

rebeccaalpert commented Dec 7, 2020

@yanpzhan - I just added the latest set of files Terry sent over in response to your comments in the newest commit. Could you please review those and then if everything looks ok, add the qe-approve label? Thanks!

@yanpzhan
Copy link
Contributor

yanpzhan commented Dec 8, 2020

/label qe-approved
Thanks all!

@openshift-ci-robot openshift-ci-robot added the qe-approved Signifies that QE has signed off on this PR label Dec 8, 2020
@spadgett
Copy link
Member

spadgett commented Dec 8, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2020
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit b8d59e1 into openshift:master Dec 8, 2020
@rh-tchuang
Copy link

@rebeccaalpert @rh-tchuang I reviewed all comments, and those updated items are good to me now, no new issue is found.
Should I add qe-approve label now or wait another review after those addressed in the translation files but not picked in for now in @rh-tfu 's comments to come in?

Thanks guys!
We're currently working on the sprint 193 batch, translation is completed for both zh/ja strings and waiting on some context clarifications; once those are addressed I'll be sending back the localized files.
Cheers

@spadgett spadgett added this to the v4.7 milestone Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console component/knative Related to knative-plugin component/kubevirt Related to kubevirt-plugin component/olm Related to OLM component/shared Related to console-shared docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet