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] num2words: add traslation to spanish of several currencies #356

Conversation

fernandahf
Copy link
Contributor

Changes proposed in this pull request:

  • In spanish, only translated in few currencies but with this change, we have more currencies to use.

Status

  • [X ] READY

How to verify this change

Run test, add three cases of added currencies.

@fernandahf fernandahf changed the title [ADD] num2words: add traslation to spanish of several currencies Add traslation to spanish of several currencies Dec 23, 2020
@fernandahf fernandahf changed the title Add traslation to spanish of several currencies [ADD] num2words: add traslation to spanish of several currencies Dec 23, 2020
@coveralls
Copy link

coveralls commented Dec 23, 2020

Pull Request Test Coverage Report for Build 968

  • 640 of 640 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 95.849%

Totals Coverage Status
Change from base Build 950: 0.5%
Covered Lines: 6003
Relevant Lines: 6187

💛 - Coveralls

@fernandahf
Copy link
Contributor Author

@moylop260

Could you review this, please?

Regards.

Copy link

@moylop260 moylop260 left a comment

Choose a reason for hiding this comment

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

@fernandahf

Could you test in spanish with all odoo currencies, please?
In order to check if we are not missing something

'PEN': (('sol', 'soles'), ('céntimo', 'céntimos')),
'CRC': (('colon', 'colones'), GENERIC_CENTS),

Choose a reason for hiding this comment

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

colón

Suggested change
'CRC': (('colon', 'colones'), GENERIC_CENTS),
'CRC': (('colón', 'colones'), GENERIC_CENTS),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch from 25e6f46 to 6426543 Compare December 26, 2020 18:42
@moylop260
Copy link

@fernandahf

I just ran the following script, check the output too in the same link:

There are errors using a few currencies.

Could you fix it, please?

tests/test_en.py Outdated
@@ -131,6 +131,194 @@ def test_to_currency(self):
"four pesos and one cent"
)

self.assertEqual(

Choose a reason for hiding this comment

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

the name of the file is test_en.py it is for English.

What about using a *es.py ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You right, it's fixed.

@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch 2 times, most recently from ab3480e to ae35a48 Compare December 30, 2020 00:24
@fernandahf
Copy link
Contributor Author

@fernandahf

I just ran the following script, check the output too in the same link:

* https://gist.github.com/moylop260/a821b38e9e98e1e9168e715a248bea76

There are errors using a few currencies.

Could you fix it, please?

I added all currencies except following:

  • BAM
  • QTQ
  • SOD

No information found about those currencies.

Regards.

@moylop260
Copy link

@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch from ae35a48 to 3290be5 Compare December 30, 2020 20:49
@fernandahf
Copy link
Contributor Author

  • QTQ

Check the data file directly from odoo:

* https://github.com/odoo/odoo/blob/b7fa0cfad03a8319f03c1aa89eb67635e1200f67/odoo/addons/base/data/res_currency_data.xml#L765

Done

@moylop260
Copy link

@erozqba @toofun666

Could you check it, please?

cc @eilst

Copy link

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

  • Traslations -> translations
  • The conventions [ADD], [FIX], etc; belon to Odoo, I'd try to conform guidelines from this project instead.

tests/test_es.py Outdated
)

TEST_CASES_TO_CURRENCY_GBP = (
(1.00, 'un libra con cero pence'),

Choose a reason for hiding this comment

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

Is "un" instead of "una" expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's not expected. It's fixed.

'YER': (('rial', 'riales'), ('fils', 'fils')),
'YUM': (('dinar', 'dinares'), ('para', 'para')),
'ZMW': (('kwacha', 'kwachas'), ('ngwee', 'ngwee')),
'ZRZ': (('zaire', 'zaire'), ('likuta', 'makuta')),

Choose a reason for hiding this comment

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

Isn't "zaires" the plural?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You right. It's fixed.

@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch 4 times, most recently from 6300de1 to 2fb7870 Compare January 7, 2021 21:28
@eilst
Copy link

eilst commented Jan 9, 2021

@lgharib Could you check it, please?

eilst
eilst previously approved these changes Jan 9, 2021
@fernandahf
Copy link
Contributor Author

@luisg123v

Could you review again, please?

Regards.

@mromdhane mromdhane self-requested a review January 21, 2021 22:48
Copy link
Collaborator

@erozqba erozqba left a comment

Choose a reason for hiding this comment

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

@fernandahf I think we don't need to loop over the CURRENCIES_UNA. Could you also please rebase your branch?

num2words/lang_ES.py Outdated Show resolved Hide resolved
@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch from 199d27d to 2841f48 Compare January 25, 2021 20:25
@fernandahf fernandahf force-pushed the master-add-traslation-spanish-currencies-fer branch from 49ee41f to b0f2ccd Compare January 25, 2021 20:35
@mrodriguezg1991 mrodriguezg1991 merged commit 400917a into savoirfairelinux:master Jan 26, 2021
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.

7 participants