-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Migrate to attributes #10113
Migrate to attributes #10113
Conversation
9b0506e
to
73bf6c3
Compare
e970087
to
01d0736
Compare
#[Column(unique: true)] protected string $name, | ||
#[Cache] #[ManyToOne(targetEntity: 'City', inversedBy: 'attractions')] #[JoinColumn(name: 'city_id', referencedColumnName: 'id')] protected City $city, |
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.
My attempt at making this more readable. Are there already PHPCS rules for that?
#[Column(unique: true)] protected string $name, | |
#[Cache] #[ManyToOne(targetEntity: 'City', inversedBy: 'attractions')] #[JoinColumn(name: 'city_id', referencedColumnName: 'id')] protected City $city, | |
#[Column(unique: true)] | |
protected string $name, | |
#[Cache] | |
#[ManyToOne(targetEntity: 'City', inversedBy: 'attractions')] | |
#[JoinColumn(name: 'city_id', referencedColumnName: 'id')] | |
protected City $city, |
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.
Ugh yeah indeed the output of Rector is not very easy on the eye here 😓 I'll try to see what phpcs rules currently exist if any.
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 did some digging and didnt find anything. Opened a feature request here - slevomat/coding-standard#1442 . Will see if we can find some resources in our company to work on this if it turns out that this is not available in some other standard already. No promises tho ;]
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.
Awesome! Thanks!
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.
@greg0ire @derrabus https://github.com/slevomat/coding-standard/releases/tag/8.6.0 have a look
<rule ref="SlevomatCodingStandard.Attributes.DisallowAttributesJoining"/>
<rule ref="SlevomatCodingStandard.Attributes.DisallowMultipleAttributesPerLine"/>
<rule ref="SlevomatCodingStandard.Attributes.AttributeAndTargetSpacing"/>
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.
Next step should be to include those in https://github.com/doctrine/coding-standard/blob/10.0.x/lib/Doctrine/ruleset.xml I guess 👍
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.
For now, let's stick with the ugly version, and then when we use doctrine/coding-standard 11, it will be pretty again?
Found while working on doctrine/orm#10113 The fix should eliminate the need for doctrine/orm@37e34e9
Found while working on doctrine/orm#10113 The fix should eliminate the need for doctrine/orm@37e34e9
Found while working on doctrine/orm#10113 The fix should eliminate the need for doctrine/orm@37e34e9
01d0736
to
96d1fd7
Compare
I used the following config: <?php declare(strict_types=1); use Rector\Config\RectorConfig; use Rector\Doctrine\Set\DoctrineSetList; return function (RectorConfig $rectorConfig): void { $rectorConfig->paths([ __DIR__ . '/tests', ]); $rectorConfig->sets([ DoctrineSetList::ANNOTATIONS_TO_ATTRIBUTES, ]); };
The tests does not behave as expected without it.
96d1fd7
to
ef4543d
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 did not look into each and every file, there's way too many of them. The files I've picked looked good. Since this affects mainly tests, I think the PR is good to merge.
TODO for a follow-up:
- Adjust UPGRADE.md because you've removed the annotation driver.
- Purge annotations from the docs.
Will do, thanks! |
No description provided.