Skip to content

Remove doctrine annotations#186

Merged
bocharsky-bw merged 7 commits intophp-translation:masterfrom
mgiraud:remove-doctrine-annotations
Mar 13, 2026
Merged

Remove doctrine annotations#186
bocharsky-bw merged 7 commits intophp-translation:masterfrom
mgiraud:remove-doctrine-annotations

Conversation

@mgiraud
Copy link
Contributor

@mgiraud mgiraud commented Mar 9, 2026

Closes #181
Closes #176

At first it thought about keeping the annotation classes, but they become useless.
Edit : Keep them anyway to avoid potential BC Break

Copy link
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

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

I have no experience with phpstan/phpdoc-parser, but I see test sare happy with this change, so it looks good to me.

Could you also fix failed SA?

@bocharsky-bw
Copy link
Member

Hm, I'm thinking about doing a minor release for this... but if we completely drop annotation files like ‎src/Annotation/Desc.php - it will be still a BC break, right? I wonder if we should revert removing them and keep them deprecated for a while in this project, along with the doctrine/annotations package? I suppose if users will not stop using them in their projects - there should not be deprecation warnings for them, right? WDYT?

@mgiraud
Copy link
Contributor Author

mgiraud commented Mar 10, 2026

I guess you are right. Some users might use them in an other way than annotations.
Will revert the removal tomorrow (or you can do it if you want). Do you have a preference on how to deprecate theses classes ?

@mgiraud
Copy link
Contributor Author

mgiraud commented Mar 11, 2026

I've have added a @deprecated tag on each annotation class.
I've considered adding trigger_error(..., E_USER_DEPRECATED); in the construct of each class but that seems overkill

Copy link
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

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

I'm fine for now with @deprecated for now, though I think if we really want to push users to get rid of it in their projects, we may need to trigger_deprecation() also, see https://symfony.com/doc/current/contributing/code/conventions.html#deprecating-code . Because simple @deprecated won't be enough to trigger a deprecation notice for users. But I think we can do it in another PR too

@mgiraud
Copy link
Contributor Author

mgiraud commented Mar 13, 2026

trigger_deprecation is great, but we need to pull another crate symfony/deprecation-contracts.
Certainly worth as there are already multiple symfony crates required by this lib.
Will do in an other PR.

@bocharsky-bw
Copy link
Member

Thank you for working on this! Let's merge

@bocharsky-bw bocharsky-bw merged commit 9098cee into php-translation:master Mar 13, 2026
5 checks passed
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.

Package doctrine/annotations is abandoned, you should avoid using it. No replacement was suggested.

2 participants