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

Add Japanese translations for Faker::Sports::Football. #2903

Merged

Conversation

yamat47
Copy link
Contributor

@yamat47 yamat47 commented Feb 12, 2024

Motivation / Background

This Pull Request is created to add new locale data for Japanese football teams, players, coaches, competition names, and positions to this library.

Additional information

This change is based on existing English data, translated by contributors who are native Japanese speakers.

To ensure the accuracy and quality of the translation, it has been reviewed by multiple native speakers knowledgeable in soccer.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug, refactor something, or add a feature.
  • Tests and Rubocop are passing before submitting your proposed changes.

If you're proposing a new generator or locale:

  • Double-check the existing generators documentation to make sure the new generator you want to add doesn't already exist.
  • You've reviewed and followed the Contributing guidelines.

Copy link
Contributor

@thdaraujo thdaraujo left a comment

Choose a reason for hiding this comment

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

nice, thank you! ⚽

ja:
faker:
football:
teams: ["レアル・マドリード", "FCバルセロナ", "バレンシアCF", "アトレティコ・マドリード", "マンチェスター・ユナイテッド", "チェルシー", "リヴァプール", "アーセナル", "トッテナム・ホットスパー", "ACミラン", "インテル・ミラノ", "ASローマ", "ユヴェントス", "バイエルン・ミュンヘン", "ボルシア・ドルトムント", "シャルケ04", "ベンフィカ", "ガラタサライ", "PSVアイントホーフェン", "LAギャラクシー", "パリ・サンジェルマン"]
Copy link
Contributor

Choose a reason for hiding this comment

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

almost forgot: do you mind converting these into dash syntax? We have instructions here: https://github.com/faker-ruby/faker/blob/main/CONTRIBUTING.md#yaml-files

You can just run this script: bundle exec rake reformat_yaml['path-to-your-file']

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've made the correction, squashed the commits, and force-pushed. Please review. Thank you!

@thdaraujo
Copy link
Contributor

hi @yamat47, could you reformat lib/locales/ja/football.yml so I can merge this?

Thanks!

@yamat47 yamat47 force-pushed the add-ja-locales-for-sport-football branch from 65587e9 to 7cbc2c5 Compare April 29, 2024 01:12
@yamat47 yamat47 requested a review from thdaraujo April 29, 2024 01:14
Copy link
Contributor

@thdaraujo thdaraujo left a comment

Choose a reason for hiding this comment

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

Thanks for reformatting!

Almost forgot - do you mind adding some tests to this and rebasing your branch? Thank you!

@yamat47 yamat47 force-pushed the add-ja-locales-for-sport-football branch 2 times, most recently from 644a6fd to 81ae746 Compare May 6, 2024 07:27
@yamat47
Copy link
Contributor Author

yamat47 commented May 6, 2024

@thdaraujo Thank you for the review.

I've added automated tests that pass locally but are failing on CI, and I'm looking into why. I used the same style as the rest of the project for RuboCop and am unclear why it's failing. Any specifics you could provide would be helpful.

Please let me know if you have any advice on how to proceed. Thank you!

Copy link
Contributor

@thdaraujo thdaraujo left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests!

To fix the Rubocop warning, you should use assert_kind_of instead.

The failing omniauth spec can be ignored because it's flaky - #2943

Could you also rebase your branch? Thanks!

def test_ja_football_methods
# The translation of "team" includes alphabets, so we cannot use assert_not_english for assertion.
# Example: "FCバルセロナ" (FC Barcelona)
assert Faker::Sports::Football.team.is_a? String
Copy link
Contributor

Choose a reason for hiding this comment

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

To fix the Rubocop warning:

Suggested change
assert Faker::Sports::Football.team.is_a? String
assert_kind_of String, Faker::Sports::Football.team

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thdaraujo Sorry for the delay in responding.

I have made the necessary corrections and force-pushed the changes. Could you please review them at your earliest convenience?

Thank you!

@yamat47 yamat47 force-pushed the add-ja-locales-for-sport-football branch from 81ae746 to 79ce64c Compare May 15, 2024 01:53
@thdaraujo thdaraujo force-pushed the add-ja-locales-for-sport-football branch from 79ce64c to 9e3f456 Compare May 15, 2024 03:41
Copy link
Contributor

@thdaraujo thdaraujo left a comment

Choose a reason for hiding this comment

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

excellent, thank you!

@thdaraujo thdaraujo merged commit 41f620d into faker-ruby:main May 15, 2024
8 checks passed
@yamat47 yamat47 deleted the add-ja-locales-for-sport-football branch May 27, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants