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

[ja] Add Japanese translation for getting started #4616

Merged
merged 8 commits into from
Jun 14, 2024

Conversation

ymotongpoo
Copy link
Contributor

@ymotongpoo ymotongpoo commented Jun 7, 2024

Preview: https://deploy-preview-4616--opentelemetry.netlify.app/ja/docs/getting-started/

@ymotongpoo
Copy link
Contributor Author

This PR needs to be merged after #4606

@ymotongpoo ymotongpoo changed the title Add Japanese translation for getting started [ja] Add Japanese translation for getting started Jun 7, 2024
@ymotongpoo ymotongpoo marked this pull request as ready for review June 8, 2024 04:58
@ymotongpoo ymotongpoo requested a review from a team June 8, 2024 04:58
@chalin
Copy link
Contributor

chalin commented Jun 8, 2024

@ymotongpoo - hmm, there are issues with this PR, as can be surmised from the many labels that the labeling bot automatically added to this PR:

image

If you can, please to the following from your local environment:

  • Ensure that youre main branch is at HEAD
  • pull this PR (you might have to do a rebase on the pull)
  • Rebase the branch of this PR from main
  • Force push the branch of this PR

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

content/ja/docs/getting-started/_index.md Outdated Show resolved Hide resolved
@chalin chalin marked this pull request as draft June 8, 2024 09:34
@ymotongpoo ymotongpoo marked this pull request as ready for review June 10, 2024 01:27
@ymotongpoo
Copy link
Contributor Author

@chalin thank you for the comment. I force-pushed this branch. I hope it should fix the labels afterwards. (I don't have permission to change labels and GH Actions hasn't changed the labels yet.)

@svrnm
Copy link
Member

svrnm commented Jun 10, 2024

@chalin thank you for the comment. I force-pushed this branch. I hope it should fix the labels afterwards. (I don't have permission to change labels and GH Actions hasn't changed the labels yet.)

I am not sure if the tooling is able to remove labels that have been set before, I manually removed them

@chalin
Copy link
Contributor

chalin commented Jun 10, 2024

@chalin thank you for the comment. I force-pushed this branch. I hope it should fix the labels afterwards. (I don't have permission to change labels and GH Actions hasn't changed the labels yet.)

Thanks. Did you rebase first?

There is still a problem with the PR in that we can't rebase from the web i/f:

image

Follow the instructions given earlier #4616 (comment):

If you can, please to the following from your local environment:

  • Ensure that youre main branch is at HEAD
  • pull this PR (you might have to do a rebase on the pull)
  • Rebase the branch of this PR from main
  • Force push the branch of this PR

The following PR has been merged and so this PR needs to be rebased because of that:

@ymotongpoo
Copy link
Contributor Author

@chalin Hmm, I rebased my fork first, rebased it with upstream/main, force-pushed then I added a couple more commits. I can't figure out why you are not able merge this PR. Let me try another force push.

@ymotongpoo
Copy link
Contributor Author

@chalin I hope this push made it work.

Comment on lines 25 to 36
- [C++](/docs/languages/cpp/)
- [.NET](/docs/languages/net/)
- [Erlang / Elixir](/docs/languages/erlang/)
- [Go](/docs/languages/go/)
- [Java](/docs/languages/java/)
- [JavaScript / TypeScript](/docs/languages/js/)
- [PHP](/docs/languages/php/)
- [Python](/docs/languages/python/)
- [Ruby](/docs/languages/ruby/)
- [Rust](/docs/languages/rust/)
- [Swift](/docs/languages/swift/)
- [Other](/docs/languages/other/)
Copy link
Contributor

@chalin chalin Jun 10, 2024

Choose a reason for hiding this comment

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

For this page and the others too, ensure that you apply the changes made via:

Otherwise, the links given here refer to the en pages, which isn't what we want.

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 see. I change them to point to ja pages.

Copy link
Contributor

@chalin chalin Jun 10, 2024

Choose a reason for hiding this comment

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

I'd rather that you use relative paths as is done in the en page. That way we keep the ja and en versions "in sync".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that #4627 was merged 2 days ago, so if I rebase onto main yesterday, the change was applied. Is it correct?

Anyway, I misunderstood your comment so I reverted the path change.

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries @ymotongpoo. Merging of translation PRs is on hold. I'll get back to you as soon as I can.

@chalin chalin force-pushed the ja/docs/getting-started/dev branch from 825a538 to 35c10b5 Compare June 10, 2024 18:29
@ymotongpoo ymotongpoo force-pushed the ja/docs/getting-started/dev branch 2 times, most recently from b6d060d to dd6e341 Compare June 11, 2024 07:09
@chalin chalin force-pushed the ja/docs/getting-started/dev branch from 44e8879 to fbefc20 Compare June 12, 2024 12:48
@ymotongpoo ymotongpoo closed this Jun 13, 2024
@ymotongpoo ymotongpoo deleted the ja/docs/getting-started/dev branch June 13, 2024 01:05
@ymotongpoo ymotongpoo restored the ja/docs/getting-started/dev branch June 13, 2024 01:08
@ymotongpoo
Copy link
Contributor Author

I deleted the remote branch by mistake. Reopening.

@ymotongpoo ymotongpoo reopened this Jun 13, 2024
@ymotongpoo
Copy link
Contributor Author

@chalin @svrnm let me check the status. my understanding is that required steps to merge this and upcoming PRs are:

  1. @katzchang gains the membership (REQUEST: New membership for @katzchang community#2146)
  2. docs-ja-approvers team will be created after 1.
  3. @katzchang (or any other approvers if others gain the membership) signs off the PR
  4. @chalin or @svrnm merges the PR

Is this correct?

@svrnm
Copy link
Member

svrnm commented Jun 13, 2024

@chalin @svrnm let me check the status. my understanding is that required steps to merge this and upcoming PRs are:

1. @katzchang gains the membership ([REQUEST: New membership for @katzchang community#2146](https://github.com/open-telemetry/community/issues/2146))

2. `docs-ja-approvers` team will be created after 1.

3. @katzchang (or any other approvers if others gain the membership) signs off the PR

4. @chalin or @svrnm merges the PR

Is this correct?

We are able to merge the PR without that being complete, we need to make this happen, but technically the PR is ready, however all translation PRs will be on hold / throttled back (not only new ones), as @chalin works on resolving core issues with the i18n-support features.

@chalin chalin force-pushed the ja/docs/getting-started/dev branch from 6e83441 to 91c4f4e Compare June 13, 2024 22:01
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

@ymotongpoo - ok, I see what is happening. You've specified that this PR is based on the English files from commit 93ab130. There have been updates to the pages in this PR since then.

Please use the following commit (or any more recent one) as your English-language file base:

You can see the diff here:

@chalin
Copy link
Contributor

chalin commented Jun 13, 2024

Also note that the Localization page instructions and script have changed. For the updated pages, see:

By the time that you are done updating this PR, when you run npm run check:i18n, there should be no ja pages listed. Ok?

@ymotongpoo
Copy link
Contributor Author

Thank you @chalin for the confirmation. Because this is the PR for files under "Getting Started", I just updated the base revision of files under getting-started directory.

Thanks to your script, I'm aware that the following files need to be updated similarly, which will be fixed in the following PRs.

> check:i18n
> scripts/check-i18n.sh

Processing paths: content
1       1       content/en/_index.md - content/ja/_index.md
30      30      content/en/docs/concepts/components.md - content/zh/docs/concepts/components.md
4       5       content/en/docs/_index.md - content/zh/docs/_index.md
1       1       content/en/_index.md - content/zh/_index.md

@chalin
Copy link
Contributor

chalin commented Jun 14, 2024

Thanks to your script, I'm aware that the following files need to be updated similarly, which will be fixed in the following PRs.

Yes, that is ok. For this PR I wanted to ensure that the Getting started pages reflected the latest en versions, which they do now, thanks for synchronizing these pages.

@chalin chalin force-pushed the ja/docs/getting-started/dev branch 2 times, most recently from 5701f63 to 8543558 Compare June 14, 2024 16:30
@chalin chalin force-pushed the ja/docs/getting-started/dev branch from 8543558 to 90e2e0b Compare June 14, 2024 22:11
@chalin
Copy link
Contributor

chalin commented Jun 14, 2024

Rebased PR that conforms to the new Translation tips. All checks are passing, so I'll merge this now.

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

👍🏻

@chalin chalin merged commit 42031fa into open-telemetry:main Jun 14, 2024
16 checks passed
@ymotongpoo ymotongpoo deleted the ja/docs/getting-started/dev branch June 17, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants