Skip to content

Modernize the codebase#492

Merged
greg0ire merged 5 commits intodoctrine:5.0.xfrom
greg0ire:modernize
Jan 22, 2026
Merged

Modernize the codebase#492
greg0ire merged 5 commits intodoctrine:5.0.xfrom
greg0ire:modernize

Conversation

@greg0ire
Copy link
Member

No description provided.

It is no longer clear to me why I did not make the
AbstractManagerRegistry::$connections and
AbstractManagerRegistry::$managers readonly already in
59029af.
The class is final and has no parent, so this is fine.
private array $connections,
private array $managers,
private readonly array $connections,
private readonly array $managers,
Copy link
Member Author

Choose a reason for hiding this comment

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

From my first commit message:

It is no longer clear to me why I did not make the
AbstractManagerRegistry::$connections and
AbstractManagerRegistry::$managers readonly already in
greg0ire@59029af.

If you can imagine a reason why I didn't, let me know. If not, then I guess the best way to find out will be to just do it.

Comment on lines 27 to 29
Copy link
Member

Choose a reason for hiding this comment

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

The @final annotation is duplicating the hard final.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, fixed!

@@ -165,6 +174,7 @@ public function getManagerNames(): array
/**
* {@inheritDoc}
Copy link
Member

Choose a reason for hiding this comment

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

should be removed. The inline phpdoc tag is only about inheriting the long description (which is only useful when wanting to include it inside the long description define of the child, thing about it as the parent:: call for the phpdoc long description).
The current usage is likely a confusion with the block-level tag @inheritDoc that is meant to say "this method is overriding the parent one which already defines everything so don't complain about missing phpdoc". And the #[Override] already provides that intent.

Copy link
Member Author

Choose a reason for hiding this comment

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

@stof should we wait on slevomat/coding-standard#1824 before attempting to address this?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR slevomat/coding-standard#1828 @greg0ire. We can remove the phpdoc in all the doctrine projects in an other PR.

@greg0ire greg0ire requested a review from GromNaN January 22, 2026 18:37
@@ -165,6 +174,7 @@ public function getManagerNames(): array
/**
* {@inheritDoc}
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR slevomat/coding-standard#1828 @greg0ire. We can remove the phpdoc in all the doctrine projects in an other PR.

@greg0ire greg0ire added this to the 5.0.0 milestone Jan 22, 2026
@greg0ire greg0ire merged commit 1f4ef63 into doctrine:5.0.x Jan 22, 2026
8 checks passed
@greg0ire greg0ire deleted the modernize branch January 22, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants