The Symfony codebase is being modernized by adding type declarations. I think that we should do the same here as well.
My suggestions:
@return annotation into an explicit return type declaration if possible. In most cases, we can do this for the 4.4 docs already.Related:
I'd love to have this ... but I'd hate to do this. Adding types at once would create millions of conflicts when merging this in all the upper branches. We don't have resources to do that, so that's why we can't do it.
However, we should start adding types to the new code and new examples added in 5.0 and higher branches.
I don't think this will create merge conflicts: (1) will be updated in all maintained versions, so there are no conflicts. For (2) and (3), it can cause some merge conflicts between 4.3 and 4.4, but this is minimal as method signatures do not change much between Symfony versions (BC promise), thus changes in these lines for 4.3 hardly conflict with change in the same line in 4.4.
I'm happy to start working on this, promoting industry best practices is the best thing we can do in the docs :).
@derrabus you said "In most cases, we can do this for the 4.4 docs already." about the return type hints. Can you explain/show some examples of which cases this is not possible? (I could not find it in symfony/symfony#33228 )
(1) will be updated in all maintained versions, so there are no conflicts.
By "all maintained versions" you also mean 3.4? Since Symfony 3.4 still supports php 5, I'm not sure if we should have code examples there using syntax exclusive to php 7.
For (2) and (3), it can cause some merge conflicts between 4.3 and 4.4, but this is minimal as method signatures do not change much between Symfony versions (BC promise), thus changes in these lines for 4.3 hardly conflict with change in the same line in 4.4.
(2) can only be done on 5.0 because the added parameter type declarations are not present in 4.4 (mainly because php 7.1 doesn't allow parameter type widening).
I'm happy to start working on this, promoting industry best practices is the best thing we can do in the docs :).
I'd also be happy to help here. Maybe we can find a way to organize/split the work here, in case others want to join as well.
@derrabus you said _"In most cases, we can do this for the 4.4 docs already."_ about the return type hints. Can you explain/show some examples of which cases this is not possible? (I could not find it in symfony/symfony#33228 )
If we add return types to implementations of Symfony interfaces, we would anticipate the return type declaration that Symfony 6 will add eventually. That's a deep dive into the crystal ball, so in cases where we're not sure how a @return will be turned into a return type declraration, we should probably not add it.
object return type would be a problem if we want to keep code snippets of the 4.4 docs compatible with php 7.1.self as return type is problematic as long as php does not support return type covariance (has been added in 7.4).In all other cases, we should be pretty safe now because on the 4.4 and 5.0 branches, Travis runs Symfony's test suite on php 7.4 against a version of Symfony that is patched with return types inferred from @return annotations.
Alright, I shortly discussed this with the rest of the team and we agreed on keeping PHP 7.1 compatible code in the Symfony 4.x docs (many people are probably still on PHP 7.1, so it's better to avoid confusion).
So this means the work that has to be done is:
I would suggest to split this work on the "topics" and to start focussing on the main guides in the root and waiting for the components docs till the very end.
Fyi, I started working on this issue in https://github.com/symfony/symfony-docs/pull/13564 . For now, the changes seems quite minor (but getting started guides mostly discuss the controller).
Related to (2) https://github.com/symfony/symfony-docs/pull/12867
Great, thanks @atailouloute!
I think you meant "@wouterj" instead? 馃構