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

Japanese translation improvement #440

Merged
merged 3 commits into from
Aug 31, 2022
Merged

Japanese translation improvement #440

merged 3 commits into from
Aug 31, 2022

Conversation

yuhattor
Copy link
Member

@yuhattor yuhattor commented Aug 21, 2022

@yuhattor yuhattor changed the title Jp translation improvement Japanese translation improvement Aug 21, 2022
@yuhattor
Copy link
Member Author

@spier Review is required, Just a quick review please:)

@yuhattor yuhattor requested a review from spier August 21, 2022 07:47
Copy link
Member

@spier spier left a comment

Choose a reason for hiding this comment

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

I can not review the Japanese itself but when you are happy with it, so am I ;)

Maybe for future PR could we get a second person from the JP patterns community to review these PRs from a pure language perspective?

Lastly, before merging this PR, please confirm if the failing link check iss as in one of your files.

@spier
Copy link
Member

spier commented Aug 21, 2022

@yuhattor please change the PR target to 'main'. All changes need to be made there first and then pushed to 'book-jp' later.

I am not at my laptop so cannot makes these changes right now. Also excuse my brevity.

I can help tomorrow to get these changes published in the book.

@yuhattor
Copy link
Member Author

yuhattor commented Aug 23, 2022

@spier

I can not review the Japanese itself but when you are happy with it, so am I ;)
Maybe for future PR could we get a second person from the JP patterns community to review these PRs from a pure language perspective?

Thank you:) Yea.. Our community in Japan is still small, but I hope we will have more contributers soon!

Lastly, before merging this PR, please confirm if the failing link check iss as in one of your files.
Sure, it looks the error is caused by GitHub Actions sending too many requests, resulting in 429.

I wonder why it failed only this time, looking at the lycheeverse/lychee settings, it looks like the default values regarding retries are applied

  ERROR	https://github.com/tapjdey/InnerSource_Project_Fitness
    Too Many Requests (HTTP error 429)

Even if I re-run it a few times, it gets rejected due to strict throttling on the GitHub side...

image

@yuhattor
Copy link
Member Author

please change the PR target to 'main'. All changes need to be made there first and then pushed to 'book-jp' later.
I am not at my laptop so cannot makes these changes right now. Also excuse my brevity.
I can help tomorrow to get these changes published in the book.

Oh, that's right. Thanks for pointing that out!

@yuhattor
Copy link
Member Author

Regarding the rate limit, it looks like I found a way to avoid it.

GitHub Token
To avoid getting rate-limited while checking GitHub links, you can optionally set an environment variable with your Github token like so GITHUB_TOKEN=xxxx, or use the --github-token CLI option. It can also be set in the config file. Here is an example config file.

https://github.com/lycheeverse/lychee

@spier
Copy link
Member

spier commented Aug 23, 2022

I believe we are already using they.
See .github/workflows/link-checker.yml

After adding that the GitHub rate limiting issues were resolved. However apparently not for good.

That token is generated by GHA on every CI run.
I suspect it has the same rate limits as a personal access token would have. Not sure though.

I say let's merge this PR into main, and watch how the link checker behaves in the future.

Regarding the rate limit, it looks like I found a way to avoid it.

GitHub Token

To avoid getting rate-limited while checking GitHub links, you can optionally set an environment variable with your Github token like so GITHUB_TOKEN=xxxx, or use the --github-token CLI option. It can also be set in the config file. Here is an example config file.

https://github.com/lycheeverse/lychee

@spier spier changed the base branch from book-jp to main August 23, 2022 15:51
@spier
Copy link
Member

spier commented Aug 23, 2022

@yuhattor I got access to a proper laptop again.

Changed the base branch of this PR to main, and approved the PR.

If this all looks good to you, please merge.

I can then help tomorrow to get this released (i.e. pushed from main to book-jp.

@spier spier added the Type - Translation Translating patterns into other languages label Aug 24, 2022
@yuhattor
Copy link
Member Author

Thank you for coordinating pull requests!!
Also, thanks for the comment about rate limits as well.
Yes, let's merge them once:)

@yuhattor yuhattor merged commit 8f1d3c8 into main Aug 31, 2022
@yuhattor yuhattor deleted the jp-translation-improvement branch August 31, 2022 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type - Translation Translating patterns into other languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants