Conversation
contributing/code/bc.rst
Outdated
| .. _note-6: | ||
|
|
||
| **[6]** Allowed using the ``@final`` annotation. | ||
| **[6]** Allowed using the ``@final since x.y`` annotation. |
There was a problem hiding this comment.
Looking at the codebase, I see that Symfony actually uses @final since Symfony x.y or @final since version x.y, but maybe this is a good time to challenge this practice and make it more universal? Or is there a purpose to mentioning Symfony here? I do not know how Symfony tooling uses @final since. Apparently there is that will cause a deprecation upon seeing @final, but I don't think it does anything with the text that follows.
There was a problem hiding this comment.
The DebugErrorHandler will include the tag description in the deprecation message (for any @final tag.
Sayig @final since Symfony 8.1 will then read The class ... is considered final since Symfony 8.1.
There was a problem hiding this comment.
OK. I've added Symfony everywhere and illustrated what you're saying with since last friday to make it clear that this is printed verbatim.
contributing/code/bc.rst
Outdated
| That is also considered a breaking change, as it reduces the list of | ||
| things that are a breaking change. |
There was a problem hiding this comment.
How is adding a phpDoc annotation a breaking change?
There was a problem hiding this comment.
If adding that annotation means it's suddenly OK to do other breaking changes, such as removing a protected property, then it is a breaking change, isn't it?
There was a problem hiding this comment.
Although, I get what you're saying, there is no way this is going to break code on its own… but I guess it breaks a promise, the promise not to alter the contract… it's a bit meta.
There was a problem hiding this comment.
I see. What you're saying makes sense, but the wording may be misleading. Adding @final itself is not a breaking change as it doesn't violate any contract, but it may lead to making actual breaking changes accidentally.
There was a problem hiding this comment.
Yes. I will try to reword this 🤔
@final since
contributing/code/bc.rst
Outdated
| For that reason, the addition of an ``@final`` annotation should never | ||
| happen in a minor or patch release, at least not in this short form. |
There was a problem hiding this comment.
The "at least not in this short form" wording implies that adding @final since in a minor or patch release is sorta okay. However, it's perfectly fine to add @final since in a minor release and is not fine at all in a patch.
It may create confusion, so I'd drop this part.
| For that reason, the addition of an ``@final`` annotation should never | ||
| happen in a minor release. |
There was a problem hiding this comment.
By the time when a reader has reached this sentence, they haven't read about @final since yet. So this sentence may read as "you cannot add any @final annotation in a minor release", but this is not what this sentence attempts to say.
We (the Doctrine team) have started using
@final sincein Doctrine, and would like some documentation about it.