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

Added Hindi Language to Time-To-leave #499

Merged
merged 9 commits into from
Oct 23, 2020
Merged

Conversation

susheelg1197
Copy link
Contributor

Related issue

Closes #475 #497

Context / Background

w.r.t #475 #497

What change is being introduced by this PR?

I have added an option of Hindi changed all the necessary files with appropriate documentation

@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

Merging #499 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #499   +/-   ##
=======================================
  Coverage   66.88%   66.88%           
=======================================
  Files          28       28           
  Lines        2564     2564           
  Branches      388      388           
=======================================
  Hits         1715     1715           
  Misses        755      755           
  Partials       94       94           
Impacted Files Coverage Δ
src/configs/app.config.js 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c48585...88511e3. Read the comment docs.

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

I found it a bit interesting that on translation.json you changed the order of the scopes, was there any specific reason for this?
My suspicion is that you have sorted alphabetically this, but this makes a bit harder when reviewing and making it consistent between all files. I don't mind the sorting in the same scope (for example, under the same $something context), as we'll be standardizing this in other languages, but for the major scopes, I would prefer to keep it in the same existing order.
Can you please adjust this?
Other than that, the changes look good, a few minor comments, but overall great. (Even though I don't know Hindi to have any opinion on the translation).

locales/hi/translation.json Outdated Show resolved Hide resolved
locales/hi/translation.json Outdated Show resolved Hide resolved
locales/hi/translation.json Outdated Show resolved Hide resolved
src/configs/app.config.js Outdated Show resolved Hide resolved
Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Oh, another thing that is missing here is to add the language "hi": "नहीं।", under the $Language scope of the translation file for the other languages, currently these should be:

  • locales/en/translation.json
  • locales/es/translation.json
  • locales/it/translation.json
  • locales/pt-BR/translation.json

@susheelg1197
Copy link
Contributor Author

Ok will do all the suggested changes

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Just a few minor issues, but should be ready for merge as soon as fixed.

locales/hi/translation.json Outdated Show resolved Hide resolved
locales/hi/translation.json Outdated Show resolved Hide resolved
@tupaschoal
Copy link
Collaborator

Note that there are some merge conflicts, please rebase and solve those. Let us know if you need help!

susheelg1197 and others added 5 commits October 22, 2020 16:50
Removed Trailing spaces
Co-authored-by: Thamara Andrade <tkcandrade@gmail.com>
Translated the word city.
Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Looks really good! Thank you a lot.
I'll be merging it soon.

Fixed proper translation for tue : "मंगल"
@thamara
Copy link
Owner

thamara commented Oct 23, 2020

Thanks! I'll continue the merge :D

@thamara
Copy link
Owner

thamara commented Oct 23, 2020

\changelog-update
Message: Translation: Time to Leave is not available in Hindi (hi)!
User: susheelg1197

@thamara thamara merged commit 88511e3 into thamara:main Oct 23, 2020
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.

Translating Time to Leave to other Languages
4 participants