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

feat(Num2Word_EU): add MX to CURRENCY_FORMS #187

Merged
merged 4 commits into from Oct 13, 2018

Conversation

temoctzin
Copy link
Contributor

Fixes # by Cuauhtémoc Díaz Minor

Changes proposed in this pull request:

  • add MX to CURRENCY_FORMS in Num2Word_EU

Status

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

How to verify this change

Additional notes

In order to avoid: NotImplementedError(u'Currency code "MXN" not implemented for "Num2Word_EN"',)

@erozqba
Copy link
Collaborator

erozqba commented Sep 2, 2018

@Cuauhtemoc-tzin thanks for taking the time to make this contribution.
Please fix this ./num2words/lang_EU.py:43:51: W291 trailing whitespace and add some test that uses MX currency with lang EU.

@ventilooo ventilooo added this to In progress in Release via automation Sep 4, 2018
@ventilooo ventilooo added this to the 1.0 milestone Sep 4, 2018
@temoctzin temoctzin force-pushed the Feat/add_MX_to_lang_EU branch 8 times, most recently from 5da7eeb to b929a49 Compare September 20, 2018 16:06
@coveralls
Copy link

coveralls commented Sep 20, 2018

Coverage Status

Coverage decreased (-3.5%) to 88.309% when pulling 3d6e31d on Cuauhtemoc-tzin:Feat/add_MX_to_lang_EU into c71d99c on savoirfairelinux:master.

@temoctzin temoctzin force-pushed the Feat/add_MX_to_lang_EU branch 3 times, most recently from be77739 to 3e8b48a Compare September 20, 2018 16:37
@temoctzin
Copy link
Contributor Author

someone can tell me, what about coverage/coveralls! plz!

@shulcsm
Copy link
Contributor

shulcsm commented Sep 21, 2018

It seems to be broken or misconfigured.

erozqba
erozqba previously approved these changes Sep 21, 2018
@erozqba
Copy link
Collaborator

erozqba commented Sep 21, 2018

@Cuauhtemoc-tzin Could you please rebase your branch over master, instead of merging master into your branch? This will make the git history more clear.
It should be easy to do:

  • First remove the merge commit: git reset HEAD~1 && git checkout .
  • Then make sure your local master branch is up to date: git fetch && git checkout master && git pull origin master
  • Then rebase your branch: git checkout Feat/add_MX_to_lang_EU && git rebase master

@erozqba
Copy link
Collaborator

erozqba commented Sep 21, 2018

About the coverage, I'm really sorry about this. I make a mistake when setting up coveralls the first time and the test/* files were included on the report. I have removed them from the report and coveralls don't update this on the right way. This should only happen on PR that was already open when I remove the test/* files from the report.

@temoctzin
Copy link
Contributor Author

rebase done!

@erozqba erozqba self-assigned this Oct 13, 2018
@coveralls
Copy link

coveralls commented Oct 13, 2018

Pull Request Test Coverage Report for Build 503

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.691%

Totals Coverage Status
Change from base Build 502: 0.0%
Covered Lines: 2471
Relevant Lines: 2755

💛 - Coveralls

@erozqba erozqba merged commit 424cf9f into savoirfairelinux:master Oct 13, 2018
Release automation moved this from In progress to Done Oct 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants