-
-
Notifications
You must be signed in to change notification settings - Fork 411
fix: EnumCaseToPascalCaseRector bugs #7272
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
2de6166
to
ec669ad
Compare
ec669ad
to
5a7cc94
Compare
|
||
return RectorConfig::configure() | ||
->withRules([EnumCaseToPascalCaseRector::class]); | ||
->withRules([EnumCaseToPascalCaseRector::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.
Like other your PR, Please don't change existing test config, use different class & config on purpose, with use of dynamic provider.
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.
In this case there's really no need for a separate config and test case just to test one fixture
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.
No, the test include existing Fixture
in autoloaded, see my next comment on removed test 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.
If you really insist then i can keep both, both the autoload fixture will have to be moved to a separate directory because the test cases were conflicting
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.
No, that on purpose :), see the whole discussion on original PRs :)
#[DataProvider('provideData')] | ||
public function test(string $filePath): void | ||
{ | ||
$this->doTestFile($filePath, true); |
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 true
2nd argument is on purpose for autoload existing Fixture directory
Closing is you change existing config/test on purpose, please don't do that. If you have specific use case to be skipped, see the differ on original PR and it configured like this :) |
Hello!
This fixes some more bugs in the rector:
self
were not being renamedThanks!