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 an Egyptian Arabic (ar_EG) locale #377

Merged
merged 13 commits into from
Nov 26, 2021
Merged

Add an Egyptian Arabic (ar_EG) locale #377

merged 13 commits into from
Nov 26, 2021

Conversation

the-ashraf
Copy link

@the-ashraf the-ashraf commented Sep 13, 2021

What is the reason for this PR?

Adds an Egyptian Arabic (ar_EG) locale

  • Egyptian cities and governorates
  • Egyptian first and last names
  • Egyptian person national id
  • Egyptian company tax id and registation numbers

Author's checklist

Summary of changes

as shown in the points above, added locale specific names, and national id number as well as company registration info.
and the all tests are passing

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

@pimjansen
Copy link

Please review the contribution guide. Will flag this as needs work until then

@the-ashraf
Copy link
Author

Addressed all the failing tasks, hopefully everything passes this time

test/Faker/Provider/ar_EG/CompanyTest.php Outdated Show resolved Hide resolved
test/Faker/Provider/ar_EG/CompanyTest.php Outdated Show resolved Hide resolved
test/Faker/Provider/ar_EG/InternetTest.php Show resolved Hide resolved
test/Faker/Provider/ar_EG/PersonTest.php Outdated Show resolved Hide resolved
test/Faker/Provider/ar_EG/PersonTest.php Show resolved Hide resolved
test/Faker/Provider/ar_EG/TextTest.php Show resolved Hide resolved
@the-ashraf
Copy link
Author

Okay, since i can't get an expected result from the arabic script based regex assertions, they pass on my local ubuntu machine, pass on a local windows machinem, yet i get conflicted resuts from pcre2 regex testers, and don't pass here. and also since they don't prove anything concise, i decided against proving that arabic text is arabic text.
i elected to remove them. coverage won't be as great. but at least this way i get expected results.

@stale
Copy link

stale bot commented Oct 4, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

@pimjansen
Copy link

Please ensure it is properly tested

@pimjansen pimjansen removed the pinned label Nov 4, 2021
@the-ashraf
Copy link
Author

Like i said, my previous tests (pcre2 regex tests) gave me conflicting results between my local dev machine and here on github. and these tests only proved that Arabic is Arabic, didn't really test something solid. and before that when i took hints from other locales in this library and made similar tests, they were cited to be changed (and rightfully so). don't think i have a valid meaningful way to test that Arabic is Arabic.

@stale
Copy link

stale bot commented Nov 23, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

@stale stale bot removed the lifecycle/stale label Nov 25, 2021
@pimjansen
Copy link

@bram-pkg lgtm so far. Can you do a final check?

@bram-pkg bram-pkg merged commit 3a097b9 into FakerPHP:main Nov 26, 2021
@the-ashraf the-ashraf deleted the ar_EG branch November 27, 2021 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants