Skip to content

Feat/add at ssn#79

Merged
pimjansen merged 13 commits intoFakerPHP:mainfrom
krsriq:FEAT/add_AT_ssn
Nov 29, 2020
Merged

Feat/add at ssn#79
pimjansen merged 13 commits intoFakerPHP:mainfrom
krsriq:FEAT/add_AT_ssn

Conversation

@krsriq
Copy link
Copy Markdown

@krsriq krsriq commented Nov 28, 2020

This PR:

  • Adds a new feature ...
  • Covered by tests
  • Fixes #nnnn

Adds Austrian social security number.

@pimjansen pimjansen added the enhancement New feature or request label Nov 28, 2020
Comment thread src/Faker/Provider/de_AT/Person.php Outdated
Comment thread src/Faker/Provider/de_AT/Person.php Outdated
@GrahamCampbell
Copy link
Copy Markdown
Member

No need to enable StyleCI on the fork. In fact, it is discouraged.

@krsriq
Copy link
Copy Markdown
Author

krsriq commented Nov 29, 2020

No need to enable StyleCI on the fork. In fact, it is discouraged.

@GrahamCampbell Sorry, was not aware of that - disabled StyleCI now on my fork.

@bram-pkg bram-pkg self-requested a review November 29, 2020 08:55
Copy link
Copy Markdown
Member

@bram-pkg bram-pkg left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment thread src/Faker/Provider/de_AT/Person.php Outdated
Signed-off-by: Daniel Schmelz <q2CIa2U7>
@pimjansen pimjansen merged commit 907b307 into FakerPHP:main Nov 29, 2020
@pimjansen
Copy link
Copy Markdown

Thanks!

@krsriq krsriq deleted the FEAT/add_AT_ssn branch November 29, 2020 12:14
Copy link
Copy Markdown

@IonBazan IonBazan left a comment

Choose a reason for hiding this comment

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

Sorry for reopening the discussion but there are some things I would improve but I missed the PR merge.

*/
public static function ssn()
{
$birthdate = (new \DateTime('@' . mt_rand(0, time())))->format('dmy');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use Faker\Provider\DateTime::dateTimeThisCentury() instead and allow to customize the birth date

$birthdate = (new \DateTime('@' . mt_rand(0, time())))->format('dmy');

do {
$consecutiveNumber = (string) mt_rand(100, 999);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can use self::numberBetween(100, 999) instead to avoid mt_rand() usage

@bram-pkg
Copy link
Copy Markdown
Member

Both suggestions sound good to me. Will you make a PR or shall I?

@krsriq
Copy link
Copy Markdown
Author

krsriq commented Nov 30, 2020

@bramceulemans I'm on it.

@bram-pkg
Copy link
Copy Markdown
Member

bram-pkg commented Dec 1, 2020

@krsriq can you also document this in https://github.com/FakerPHP/fakerphp.github.io?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants