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 es_PE person #1844

Closed
wants to merge 11 commits into from
Closed

Conversation

JsonVladimir
Copy link

Added es_PE to person, I recently used the library and noticed that there were no common names from Peru, I decided to propose that the file be added.

Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

Could you add some test cases for the new providers?

Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

Can yuou fix the remaining linting issue? You can run make lint to format using the project's settings.

@JsonVladimir
Copy link
Author

JsonVladimir commented Apr 26, 2023

Can yuou fix the remaining linting issue? You can run make lint to format using the project's settings.

@fcurella When I run black it tells me there is 1 file that needs to be reformatted but when i format it it tells me no changes were made.

C:\GitHub\faker> python -m black --check --line-length 120 C:\GitHub\faker
would reformat C:\GitHub\faker\tests\providers\test_phone_number.py

Oh no! 💥 💔 💥
1 file would be reformatted, 683 files would be left unchanged.
C:\GitHub\faker> python -m black C:\GitHub\faker\tests\providers\test_phone_number.py
**All done! ✨ 🍰 ✨
1 file _left unchanged_.**

image
image

@fcurella
Copy link
Collaborator

@JsonVladimir I'm not seeing --line-length 120 on the second command you called. You can just run make lint and it should run it with the correct arguments for you

Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

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

Could you check the failing tests?

iban = faker.iban()
assert is_valid_iban(iban)
assert iban[:2] == EsPeBankProvider.country_code
assert re.fullmatch(r"\d{2}[A-Z]{4}}\d{18}", iban[2:])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I see an extra } in {4}}

tests/providers/test_phone_number.py Outdated Show resolved Hide resolved
class TestEsPE:
"""Test es_PE phone number provider methods"""

pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we actually test something? :P

@github-actions github-actions bot added the stale label Aug 15, 2023
@github-actions github-actions bot closed this Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants