Changed private static properties to private const #38213
Conversation
96f279a to
8834baa
Compare
|
changing this only in master will make maintenance harder (we will get potential conflicts on all these places for each merge-up), while this provides little benefit. As this applies only to private stuff, this can be changed at any time without any BC impact. So if we decide to apply such change on existing code, I would suggest doing that in the oldest supported branch, so that all maintained branches have that and don't conflict when merging up later. But given |
|
@stof Whenever you decide to release this. Please let me also know if this is something you support. |
2c6c101 to
d16a487
Compare
|
I agree that this would be a reasonable code modernization. If we do it, the patch should be applied to the 4.4 branch after the final bugfix release of the 3.4 branch. |
|
Ready for review generally. |
|
An example which public static properties to potentially change is: The backward compatibility is rather easy as it is possible to assign the value of the new const to the static property. I admit changing this with the necessary deprecation means a lot of documentation for this rather small change and is maybe not reasonable. On the other hand it is the only way to get rid of it with the clean upgrade strategy Symfony has. |
it even involves replacing the public property with a magic one. Otherwise, we have no way to trigger deprecations when using it. |
|
Yes, if there is a requirement of a thrown message this is necessary. But that's not a much higher complexity. |
|
@stof soft-deprecating public static props would work, isnt it? AFAIK we cant even overload static props in php ..., so my proposal would be /** @deprecated use class::X instead */
public static $x = self::X;
public const X = ...;
``` |
|
@ro0NL we cannot warn for such deprecation, so it might not be enough for projects to migrate. And indeed, for static props, the trick of the magic property won't work. |
|
I think this might be worth doing in 4.4 immediately. I don't think 3.4 will evolute that much around those lines in the next 3 months. |
we could try :) |
|
If you accept it for 4.4 at this point I'll redo it on this branch. Let me know if this has been decided. Regarding public/protected deprecation: |
|
if @nicolas-grekas is OK with doing it now in 4.4 rather than waiting, this is fine with me. He is the one doing the merge-up regularly, not me, so I will accept it if he thinks that the 2 months delay is not necessary. |
|
Let’s wait the end of 3.4 first. |
…erger) This PR was merged into the 4.4 branch. Discussion ---------- Changed private static array-properties to const | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | Deprecations? | no | License | MIT This changes all private static properties (just screened arrays) to private const. I left out those which have write access obviously and also those with `static::` access. It is a new implementation of #38213. Based on 4.4 this time. If merging it up creates several conflicts, you could ignore all changes of 4.4 and I create a new branch for 5.1 or 5.2. I'll do this anyway if any new private static arrays are existing there. Commits ------- aa79381 Changed private static array-properties to const
This PR changes all private static properties with read-only usage to private const.