Skip to content
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

feat: @Symfony - keep Annotation,NamedArgumentConstructor,Target annotations as single group #7399

Merged
merged 5 commits into from Oct 31, 2023

Conversation

keradus
Copy link
Member

@keradus keradus commented Oct 31, 2023

@keradus keradus marked this pull request as ready for review October 31, 2023 15:40
'phpdoc_separation' => [
'groups' => [
...PhpdocSeparationFixer::OPTION_GROUPS_DEFAULT,
['Annotation', 'NamedArgumentConstructor', 'Target'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that grouping those would also belong to a doctrine annotations ruleset, as they relate to the usage of doctrine annotations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its valid point,
yet phpdoc_separation is not currently in use by @DoctrineAnnotation and I'm not sure if we want to start using it there.

There is tricky point of rule getting override if multiple rulesets are using it, and maybe it's not a top priority to solve it now. Doctrine ruleset could benefit from even bigger cleanup - especially about annotations -> attributes.
I'm not user of Doctrine and won't be able to judge there. Maybe we improve ruleset of Sf in first step, and if anyone would like to also take closer look to improve ruleset of Doctrine, we can have that as follow-up PRs (also very open if you are up for implementing something for Doctrine settings/rules)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the @DoctrineAnnotation is about doctrine/annotations IMO, so it should not care about attributes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it's probably like who effort to cleanup current rules, create rules that migrate anno to attrib, make sure rules works for both (or have sibling rule), ....
big thing I don't wan to mess myself, as not knowing that ecosystem.

@coveralls
Copy link

Coverage Status

coverage: 94.604%. remained the same when pulling a83d808 on keradus:xx into 877bfb6 on PHP-CS-Fixer:master.

@keradus keradus merged commit 24cf2a0 into PHP-CS-Fixer:master Oct 31, 2023
15 checks passed
@keradus keradus deleted the xx branch October 31, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants