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

Translations for ja.yml through line 175 #1243

Merged
merged 8 commits into from
Nov 8, 2018
Merged

Translations for ja.yml through line 175 #1243

merged 8 commits into from
Nov 8, 2018

Conversation

toryalsip
Copy link
Contributor

Description

I think the first 175 lines here were for the most part fairly straightforward, but I'm sure there are some mistakes. I put my wife's GitHub handle as co-author as she helped a little with some of these.

Corresponding Issue

Relates to #1215

Test Coverage

@toryalsip toryalsip requested a review from nshki November 4, 2018 17:38
config/locales/ja.yml Outdated Show resolved Hide resolved
@julianguyen
Copy link
Member

Could you rebase from master? There are some new translations added related to the report feature. Sorry about that!

@toryalsip
Copy link
Contributor Author

@julianguyen yeah, I could rebase but I think that would be somewhat disruptive to the flow. Would it be better if I just merge master into ja-translations (and fix conflicts) and from ja-translations to the branch related to this review? I'd personally prefer to do any rebasing once we're near the end and I can just tidy up all commits prior to merging into master at that time.

@julianguyen
Copy link
Member

@matcha440 That works! :)

Copy link
Member

@nshki nshki left a comment

Choose a reason for hiding this comment

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

This is a great start! Unfortunately didn't have a ton of time to review all translations, but I left a few comments as a first pass.

config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
@toryalsip
Copy link
Contributor Author

@nshki thank you for the initial feedback. I'll get these updated a little later today but had a question on one of them.

Also resolve merge conflicts to add new reports section to ja.yml
@toryalsip
Copy link
Contributor Author

Merging things in from master added some new entries, but I'll save those for later to keep the scope of this PR focused.

Copy link
Member

@nshki nshki left a comment

Choose a reason for hiding this comment

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

Thanks again for these. Finally made a second pass, and left a few more comments. Would be great if you could have your wife take a look through this file's translations as well!

config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
config/locales/ja.yml Outdated Show resolved Hide resolved
@toryalsip toryalsip merged commit b5f73e4 into ifmeorg:ja-translations Nov 8, 2018
@toryalsip toryalsip deleted the ja-yml-through-175 branch November 8, 2018 16:12
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.

4 participants