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 support for Hindi language #607

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

bryananderson
Copy link
Contributor

Copied from stale PR here: #442

This is 100% the work of @pablohonney; that PR is over two years stale so I just started a fresh branch with the relevant parts and resolved the flake8/isort problems.

Changes proposed in this pull request:

  • add support for Hindi language
  • add tests

Status

  • READY
  • HOLD
  • WIP (Work-In-Progress)

How to verify this change

run tests

@coveralls
Copy link

coveralls commented Dec 17, 2024

Coverage Status

coverage: 98.082% (+0.01%) from 98.07%
when pulling d3aca75 on bryananderson:add-hindi
into beca211 on savoirfairelinux:master.

@bryananderson
Copy link
Contributor Author

Coveralls is failing because this line isn't covered. However, this line is currently unreachable for Hindi because all integers < 100 are in self.cards, so there is never a case where we need to merge two integers below 100.

Let me know if you prefer me to remove this elif altogether due to reduced coverage, or leave it in case future changes to lang_HI.py make the above assumption no longer hold.

@mrodriguezg1991
Copy link
Contributor

Coveralls is failing because this line isn't covered. However, this line is currently unreachable for Hindi because all integers < 100 are in self.cards, so there is never a case where we need to merge two integers below 100.

Let me know if you prefer me to remove this elif altogether due to reduced coverage, or leave it in case future changes to lang_HI.py make the above assumption no longer hold.

If there is not real scenario where this conditional is trigger then the elif should be remove to avoid the test decrease.
Thanks for contributing to the project

@mrodriguezg1991
Copy link
Contributor

Also please remove the .coverage file from the PR, thanks

@bryananderson bryananderson force-pushed the add-hindi branch 3 times, most recently from 6ba9b10 to ec62f26 Compare January 8, 2025 08:59
@bryananderson
Copy link
Contributor Author

Removed that line and the coverage file!

@bryananderson
Copy link
Contributor Author

@mrodriguezg1991 believe this one is ready to go too

@mrodriguezg1991 mrodriguezg1991 merged commit 5267ce1 into savoirfairelinux:master Jan 9, 2025
14 checks passed
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.

3 participants