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 coverage tests for many countries #633

Merged
merged 8 commits into from
Jul 23, 2022

Conversation

akosfurton
Copy link
Contributor

@akosfurton akosfurton commented Feb 23, 2022

Add many other country tests to fix edge cases and improve coverage

@dr-prodigy , please approve the workflows

@akosfurton akosfurton changed the title Add coverage tests for Honduras Add coverage tests for many countries Feb 23, 2022
Copy link
Collaborator

@dr-prodigy dr-prodigy left a comment

Choose a reason for hiding this comment

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

Hi @akosfurton !
While I agree on a good number of these reviews, I don't understand why there are a lot of "elif" blocks being replaced by "if".. which is maybe not functionally harmful, but surely less ideal in terms of performance..
could you please explain their reason why and/or review them?
thx!

@dr-prodigy
Copy link
Collaborator

PS: btw @akosfurton , relevant build tests are also failing (most probably due to some mis-formatting): could you double-check those too?
Thx! 👍

@akosfurton
Copy link
Contributor Author

Yes, will make the above changes and re-run the tests

@dr-prodigy
Copy link
Collaborator

hey there @akosfurton any news on what above?
thank you very much, KUTGW 👍

@dr-prodigy dr-prodigy merged commit 2b3abe1 into vacanza:beta Jul 23, 2022
@dr-prodigy
Copy link
Collaborator

At last, I applied this PR too, after some final fixes. Now available in beta, thx 👍

@dr-prodigy dr-prodigy mentioned this pull request Aug 21, 2022
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.

2 participants