Redesign client_info and client_version in mysqli#6777
Redesign client_info and client_version in mysqli#6777kamil-tekiela wants to merge 6 commits intophp:masterfrom
Conversation
ext/mysqli/mysqli.stub.php
Outdated
|
|
||
| final class mysqli_driver | ||
| { | ||
| /** @deprecated 8.1.0 */ |
There was a problem hiding this comment.
Generally, something is only deprecated after the new preferred usage is added. See #6754 for a similar example.
There was a problem hiding this comment.
We could do that too. Generally, I agree that there should be first an alternate way and then deprecation, but in this case there already is an alternate way. If you think that this is a good idea then we can deprecate mysqli::get_client_info() now and deprecate properties in PHP 8.2. Not sure it will make much difference here. I just didn't want to add a fifth way of accessing these values without clearly marking which ones will be made obsolete.
There was a problem hiding this comment.
To be honest, I don't have any strong preference, because your reasoning makes sense for me.
|
I feel like this makes for unnecessary churn for some functionality nobody really needs anyway. Can we keep this only at the first two bullet points? That is, keep the functions, keep the properties, drop the OO method and arg? That should arrive at a consistent interface. |
e5d24eb to
4dda04a
Compare
|
@nikic The idea was to slowly fix all minor problems with mysqli. Having constants as read-only properties is certainly not desirable. I think that some people are using these constants in their code to check for the availability of certain functions. E.g. I added a new commit that reverts all changes to the properties and the addition of new constants. But it makes no sense to me to keep them as object properties if they are never dependent on the object. And I definitely see no reason to have them as properties of |
nikic
left a comment
There was a problem hiding this comment.
Please also add UPGRADING notes to the deprecated functionality section.
Co-authored-by: Nikita Popov <nikita.ppv@googlemail.com> Fix P
d68f63a to
c777a4a
Compare
…eter for `mysqli_get_client_info()` > Calling `mysqli::get_client_info()` or `mysqli_get_client_info()` with the `mysqli` argument has been deprecated. Call `mysqli_get_client_info()` without any arguments to obtain the version information of the client library. Includes unit tests. Refs: * https://github.com/php/php-src/blob/3a71fcf5caf042a4ce8a586a6b554fd70432e1e2/UPGRADING#L423-L426 * https://www.php.net/manual/en/migration81.deprecated.php#migration81.deprecated.mysqli * php/php-src#6777 * php/php-src@7e9f6d2
There are two constants defined in the MySQL client API:
client_info(string) andclient_version(int). In MySQLnd these constants are pegged to the PHP version. In libmysql they represent the client library version used in compilation.Until now, mysqli was exposing these constants in 4 different ways:
mysqli_driverproperties,mysqliproperties,mysqli_get_client_info()function, andmysqli::get_client_infomethod. There was no method forclient_versionthough.The C MySQL library exposes these constants only in 2 ways to PHP: using a constant and using a function call. For this reason, I am proposing to unify the access method from mysqli to the same two options: a constant and a function call.
What is in this PR:
mysqli_get_client_info()mysqli::get_client_infomethodmysqliandmysqli_driverclass.mysqliandmysqli_driverclass.The goal is to remove
mysqli_driverclass at some point if possible, therefore the new constant is made redundant in both classes, but it is also possible to add it only tomysqliclass.