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

Fix greek phone numbers #16

Merged
merged 4 commits into from
Nov 27, 2020
Merged

Fix greek phone numbers #16

merged 4 commits into from
Nov 27, 2020

Conversation

sebdesign
Copy link

This PR fixes the greek phone numbers, because the existing phone numbers are not valid according to libphonenumber.

I've used the data from Wikipedia and the regular expressions from giggsey/libphonenumber-for-php to generate valid greek numbers.

The tests are not extensive but I ran them 10000 times using phpunit test/Faker/Provider/el_GR/PhoneNumberTest.php --repeat 10000 and they all pass.

@pimjansen pimjansen changed the base branch from 1.10 to master October 28, 2020 07:55
@pimjansen pimjansen added the bug Something isn't working label Oct 28, 2020
@localheinz localheinz changed the base branch from master to main October 28, 2020 08:10
@pimjansen
Copy link

Can we add some PHPDoc in the new methods? I know its not there everywhere but it will help us future wise a lot

@sebdesign
Copy link
Author

@pimjansen I've added return types as well as docblocks with descriptions and examples.

@pimjansen
Copy link

@sebdesign, you introduced a breaking change by setting up the return types.

@sebdesign
Copy link
Author

Ah, you're right, I removed them now.

@sebdesign
Copy link
Author

@pimjansen BC check is still squawking because i changed some methods from static to non-static.

I suppose this is not an issue, because the the provider methods would never be called directly, but only via Generator::format(), right?

@pimjansen
Copy link

Its not a final close so you change the interface of the actual and therefor it is breaking

@sebdesign
Copy link
Author

If I make the methods static I won't be able to use $this->generator->parse(). I'll have to find a workaround to replace {{formatter}} with actual values. Is that ok with you?

@bram-pkg
Copy link
Member

You can use string interpolation.

@bram-pkg
Copy link
Member

Looks good to me.

@pimjansen pimjansen merged commit 9f1d7e8 into FakerPHP:main Nov 27, 2020
@pimjansen
Copy link

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants