Added Person Faker for en_PH #1463
Conversation
|
Why the checks failed? I don't think the failures got something related on my commit. |
|
Check out the CONTRIBUTING.md |
|
@jcfrane Please close and reopen this pull request to build again |
|
I will thanks. |
|
@Saibamen still failing... Some checks actually passed. Hmmmm. |
|
@jcfrane But now this is only your fault. See PHPUnit info: |
|
@Saibamen I see, Ok i'll fix this. |
| 'Llorca', 'Panalangin', 'Dionisio', 'Cruz', 'Esteban', 'Santos', 'Aquino' | ||
| ); | ||
|
|
||
| protected static $suffix = array('Jr.', 'Sr.', 'I', 'II', 'III', 'IV', 'V', 'MD', 'PhD', ); |
There was a problem hiding this comment.
It's not enough to introduce a new format, you need to introduce a method as well. See src/Faker/Provider/Person.php, every static property has a corresponding (static) method. Some tests pass because the randomness doesn't always include the suffix format.
| 'Anthony', 'Allan', | ||
| 'Ben', 'Buboy', 'Bobby', 'Bentong', 'Bayani', | ||
| 'Cydrick', 'Carlo', 'Cardo', 'Cesar', | ||
| 'JC', 'Mark Anthony', 'Jayfer', 'Paolo', 'Poy', 'Warren', 'Stephen', 'Maynard', |
There was a problem hiding this comment.
There is a double whitespace here.
| '{{firstNameFemale}} {{lastName}}', | ||
| ); | ||
|
|
||
| protected static $firstNameMale = array( |
There was a problem hiding this comment.
Please order this alphabetically.
| 'Michael', 'Jeric', 'Rex', 'Benigno' | ||
| ); | ||
|
|
||
| protected static $firstNameFemale = array( |
There was a problem hiding this comment.
Please order this alphabetically. Lines should be 80 characters or less, this makes a diff easier to read, structure your code accordingly.
| ); | ||
|
|
||
| protected static $lastName = array( | ||
| 'Frane', 'Rosario', 'Nonog', 'Balecha', 'Abendanio', 'Ramilio', 'Lacerna', 'Villanueva', 'Padilla', |
|
@ppelgrims i'll fix all your comments. Thanks for the guide :) My first time to actually contribute on open source. |
|
@jcfrane bump, could you please review again so we can find a way to get it merged? |
|
I am going to close this PR as stale. If this still needs to be merged, please feel free to reopen it. |
No description provided.