Adding back some removed functions for PHP 8 (adding as deprecated) #320
Conversation
Codecov Report
@@ Coverage Diff @@
## master #320 +/- ##
=========================================
Coverage 48.48% 48.48%
Complexity 308 308
=========================================
Files 15 15
Lines 794 794
=========================================
Hits 385 385
Misses 409 409 Continue to review full report at Codecov.
|
|
Hello jaydiablo First, yes adding @deprecated tags is a good idea, I would very much like you add to add them to the other functions. About the risk of stepping on someone else toes, you might like to talk to @nusje2000 about #308, however this PR looks kind of dead. How urgently do you think you need theses functions? And how long do you wish we maintained them? |
|
This was released with the tag 2.1.0 |
|
@Kharhamel thanks for merging! I can make a separate PR to add more deprecated tags and possibly add additional functions if I find any. Regarding your question about how long these should be maintained, I suppose that's probably best aligned with how long PHP 7.4 remains actively maintained (I believe it's EOL at the end of this year (it's in security support now, no other releases)), so probably until then? I don't quite know the strategy for this lib in terms of how it's going to handle new PHP versions over time. Will there be a 3.x release for PHP 8.1? Will the same strategy be applied there, but only support changes between PHP 8.0 and 8.1 in that release (the 7.4 functions get removed in 3.x)? Thanks again for the merge and release. |
|
Sorry @jaydiablo didn't see your response
There is no definitive strategy. Since the php documentation on github only has one update stream and no tags, and this project is synced on this doc, we are basically forced to always support the last version. The alternative is to commit the documentation folder at some point to "freeze" the project at a specific php version (which is what @dbrekelmans did at some point for php7.4, don't know if it is still going) but i don't think this is scalable if we want to support every minor versions. The original goal was to never update to php8 in the first place, and instead update the php core itself to fix all theses return values so that this kind of library wouldn't be needed. |
I believe this references #307.
We use Safe in our code base and have recently been upgrading to PHP 8.0 in some of our apps, which required bumping Safe to 2.0 to deal with an issue with DateTimeImmutable (fixed in #298), however, this revealed that Safe 2.0 doesn't have some of the functions that 1.x does since they were unnecessary in PHP 8 due to upstream signature changes.
To get these apps to work, I needed to add these functions to Safe (because some of the code is shared between 7.4 and 8.0 applications, we can't just remove the
use function Safe\*statements).I also added a
@deprecatedannotation to the functions I added (otherwise they're direct copies from the 1.3.3 tag) so that users of PHPStan or Psalm can be notified of the deprecation and deal with removing them (or not) in their code base.I know there's been some discussion about diffing the generated functions between PHP versions for #317, this PR doesn't do that (I just searched for cases that affected our code), but could be added to it if it's decided that this is the approach the project would like to take for this.
Let me know if this is useful as-is and also if the
@deprecatedannotation is a good approach or not (I noticed the other deprecated functions do not have this annotation, so I didn't add it there, but happy to add if that makes sense).Also, I don't mean to step on any toes if this work is already in progress. We needed the changes in place for our own code base so figured I'd try to contribute back since the work was already done. Feel free to close this out if there are plans to do this differently, we can maintain our fork until this is addressed more formally in the project.
Thanks!