-
Notifications
You must be signed in to change notification settings - Fork 101
[SYMFONY 4.4] replace Link argument values with constants #812
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
use Rector\Config\RectorConfig; | ||
|
||
// https://github.com/symfony/symfony/blob/4.4/UPGRADE-4.4.md | ||
|
||
return static function (RectorConfig $rectorConfig): void { | ||
$rectorConfig->ruleWithConfiguration(ReplaceArgumentDefaultValueRector::class, [ |
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.
This should import file
symfony44-weblink.php
then include here and in Set class
https://github.com/rectorphp/rector-symfony/blob/main/src/Set/SetProvider/Symfony4SetProvider.php
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.
Done 👍
@@ -12,4 +12,5 @@ | |||
$rectorConfig->import(__DIR__ . '/symfony44/symfony44-http-kernel.php'); | |||
$rectorConfig->import(__DIR__ . '/symfony44/symfony44-templating.php'); | |||
$rectorConfig->import(__DIR__ . '/symfony44/symfony44-security-core.php'); | |||
$rectorConfig->import(__DIR__ . '/symfony44/symfony44-weblink.php'); |
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.
Please register to provider Symfony4SetProvider
https://github.com/rectorphp/rector-symfony/blob/main/src/Set/SetProvider/Symfony4SetProvider.php
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.
Done
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.
@samsonasik i see we have:
new ComposerTriggeredSet(
SetGroup::SYMFONY,
'symfony/symfony',
'4.4',
__DIR__ . '/../../../config/sets/symfony/symfony4/symfony44.php'
),
In the list and symfony44.php'
has all the sub includes like mine symfony44-web-link.php
and also symfony44-templating.php
is this expected? Because you might not want to provide symfony44-web-link.php
when you dont have the symfony/web-link
package right? Should it be in symfony44.php'
?
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.
that's for upgrade by composer per component, so something like, see other list
new ComposerTriggeredSet(
SetGroup::SYMFONY,
'symfony/weblink',
'4.4',
__DIR__ . '/../../../config/sets/symfony/symfony4/symfony44-weblink,php'
),
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.
Check, i added that i was just wondering because it is also already in list config/sets/symfony/symfony4/symfony44.php
so might be added when you dont have symfony/weblink
or not?
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.
that will auto detect when you have symfony/weblink in composer.json by component
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.
Check, it should be ready for review i also updated the other pull requests
Thank you @JohJohan |
On hold see: rectorphp/rector-src#7068