-
-
Notifications
You must be signed in to change notification settings - Fork 723
Rector as Services to remove dump proxy "rectors" to "services" mechanism #335
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
Conversation
@TomasVotruba Gonna review this afternoon. Sounds like an amazing improvement. |
861b9e9
to
7476315
Compare
…and service logic
7476315
to
23c9765
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will improve better understand of Rector as well 👏
Thank you, it definitely will 👍 |
# old namespace prefix | ||
- 'PHPUnit_' | ||
# exclude classes | ||
- '!PHPUnit_Framework_MockObject_MockObject' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it could be another param with explicit name, like $skipClasses
To show this feature to the user without commenting everything
Makes use of this awesome feature: symfony/symfony#21313 |
How to: custom service formats in
|
Why?
using
rectors
key in config instead ofservices
was just to save 1 extra line of config:Before
After
Which of these 2 version is clear to first time user, that meets Rector the first time?
To make this work 5 extra classes were needed, that duplicated Symfony DI:
I don't like reinventing the wheel, moreover in Symfony\DI case
I also recall, how frustrated I was when writing ECS and exploring PHP_CodeSniffer and PHP CS Fixer when I found out - all sniffs/fixers are just statically added services; it would save me dozens of hours if
services
like here would be usedAlso writing a post about Rector and Collector helped me to realize this WTF
For all these reasons we should get rid of this extra layer that only came from historical background now and adds more comlexity then help.
Benefits
Todo