Skip to content

Reset formatters when adding a new provider#386

Merged
bram-pkg merged 1 commit intoFakerPHP:mainfrom
aanfarhan:main
Dec 21, 2021
Merged

Reset formatters when adding a new provider#386
bram-pkg merged 1 commit intoFakerPHP:mainfrom
aanfarhan:main

Conversation

@aanfarhan
Copy link
Copy Markdown

@aanfarhan aanfarhan commented Nov 2, 2021

What is the reason for this PR?

Fixed bug #366
This bug occurred when you add new Provider after calling the previous provider formatter with the same function name as the new one. To explain it more clearly read this code:

// Existing behaviour
$faker = Faker\Factory::create(); 
$faker->firstName(); // calling default provider : Faker\Provider\en_US\Person
$faker->addProvider(new Faker\Provider\zh_CN\Person($faker));
$faker->firstName(); // still calling the default provider because of this code https://github.com/FakerPHP/Faker/blob/d3c0752e756a387214c5b942cb7c5231d9bc8b64/src/Faker/Generator.php#L632

// What this PR does
$faker = Faker\Factory::create(); 
$faker->firstName(); // calling default provider : Faker\Provider\en_US\Person
$faker->addProvider(new Faker\Provider\zh_CN\Person($faker));
$faker->firstName(); // calling the new provider formatter

Author's checklist

Summary of changes

Remove existing formatters that match the new Provider formatters when calling addProvider function.

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

@aanfarhan aanfarhan requested a review from Nyholm November 2, 2021 06:31
@stale
Copy link
Copy Markdown

stale bot commented Nov 16, 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
Copy link
Copy Markdown

stale bot commented Dec 1, 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.

@otilor
Copy link
Copy Markdown

otilor commented Dec 21, 2021

Anyone reviewing this?

@bram-pkg
Copy link
Copy Markdown
Member

Yes, sorry, this PR got swept under my radar.

Could you rebase on main so the BC Roave check doesn't break?

@otilor
Copy link
Copy Markdown

otilor commented Dec 21, 2021

@aanfarhan kindly rebase on main.

@localheinz localheinz force-pushed the main branch 5 times, most recently from 8d2b436 to 46ace5f Compare December 21, 2021 11:02
Copy link
Copy Markdown
Member

@localheinz localheinz left a comment

Choose a reason for hiding this comment

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

👍

@localheinz localheinz changed the title Fix bug | remove previous provider's formatter when addProvider funct… Reset formatters when adding a new provider Dec 21, 2021
@localheinz localheinz added bug Something isn't working and removed pinned labels Dec 21, 2021
…ion called if the current formatter exists in the new added provider
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.

LGTM!

@bram-pkg bram-pkg merged commit 47693c7 into FakerPHP:main Dec 21, 2021
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.

Formatters are not reset when adding a new provider

5 participants