Deprecate property access #164
Conversation
Nyholm
force-pushed
the
deprecate-property
branch
from
651ad18 to
29bc1ec
Compare
| @@ -6,12 +6,18 @@ | |||
|
|
|||
| final class DefaultGeneratorTest extends TestCase | |||
| { | |||
| /** | |||
| * @group legacy | |||
There was a problem hiding this comment.
As far as I can see, we do not run different groups of tests at the moment.
What is the benefit of adding this annotation, then?
There was a problem hiding this comment.
The symfony/phpunit-bridge has a super power, its default configuration (which we use) will fail the test if we use deprecated code... unless we mark the test as legacy.
There was a problem hiding this comment.
Sounds more like magic to me, to be honest!
There was a problem hiding this comment.
Sounds more like magic to me, to be honest!
Maybe it is. ¯_(ツ)_/¯
Here are some related docs: https://symfony.com/doc/current/components/phpunit_bridge.html#mark-tests-as-legacy
No matter if it is magic or not, it is an excellent feature. It can help us to be sure that we never use code that is deprecated. So, if our code is tested, a user may always be able to use Faker without triggering a deprecation notice.
This will also help us to to know what code and tests we should remove before tagging 2.0
There was a problem hiding this comment.
I'm sure we would find out via static code analysis, so I think we do not need this.
There was a problem hiding this comment.
But since you added it and we can easily remove it with the general_phpdoc_annotation_remove fixer, I think we can keep it in for now.
Nyholm
force-pushed
the
deprecate-property
branch
from
29bc1ec to
369975c
Compare
Nyholm
force-pushed
the
deprecate-property
branch
from
369975c to
959f5ae
Compare
|
Thank you, @Nyholm and @pimjansen! |
|
Thank you for merging. |
In concordance with [laravel#5583](laravel/laravel#5583), as after Faker PHP 1.14, using properties is deprecated, and it is recommended to use methods instead. See [laravel#164](FakerPHP/Faker#164) for details.
In concordance with [laravel#5583](laravel/laravel#5583), as after Faker PHP 1.14, using properties is deprecated, and it is recommended to use methods instead. See [laravel#164](FakerPHP/Faker#164) for details.
* [8.x] Using faker methods instead of properties In concordance with [#5583](laravel/laravel#5583), as after Faker PHP 1.14, using properties is deprecated, and it is recommended to use methods instead. See [#164](FakerPHP/Faker#164) for details. * [8.x] Using faker method instead of properties In concordance with [#5583](laravel/laravel#5583), as after Faker PHP 1.14, using properties is deprecated, and it is recommended to use methods instead. See [#164](FakerPHP/Faker#164) for details.
This PR deprecates access to
$faker->name. One should always use$faker->name()instead.I also add
@group legacyto all tests for the providers. This is too many, I know, but I'll soon deprecate all providers too. =)Why we should use
trigger_deprecation()and not writing the deprecations ourselves? Some benefits are described in the readme. But the major thing for me is that deprecations will be easier to write (and eventually remove).This is just the first of the many deprecations we will add the next few weeks.