FAQ
Bump.

How did this end?

-Hannes

On Sun, Apr 13, 2014 at 1:50 PM, Bob Weinand wrote:
Am 13.4.2014 um 22:32 schrieb Stas Malyshev <smalyshev@sugarcrm.com>:
Hi!
an API change. The interface hasn't changed.
It has. Formerly, you did not have a parameter supplied for this
function, starting with next version, you do. If you write code that
implements this function, you need to know it changed.

Also, this change seems to break a lot of tests, see:
https://travis-ci.org/php/php-src/builds/22895161

Looks like it is not as BC safe as it was intended. Please fix it or
revert the change - BC break in 5.4 is not acceptable.
The tests fail, because Zend checks for the number of parameters passed and warns then.
It'd only be an API change if it forced the user to change its declaration.
Not true. For example, we document new functions in UPGRADING, even
though adding new functions does not force anybody to change anything.
Same with new parameter to existing functions - see for example "Changes
to existing functions" section.
I thought UPGRADING would be only for new versions (not micros).
Looking at it again, I don't think we can put this patch in either 5.4
or 5.5. It changes extension API and breaks any module that implements
Countable because most of them expect 0 params and will produce warning
if given one param. Right now it broke Phar and SplObserver, but
third-party modules will have the same issue. Unless you find a way to
fix this in a generic way, we can only do it in 5.6, and even that only
with permission from Ferenc, since we're in the freeze now. In any case,
this means UPGRADING and probably UPGRADING.INTERNALS note is absolutely
necessary, as every extension having Countable classes will need to be
fixed.
Btw. I really didn't think about the fact that zpp warns if there are too many
parameters. I think your concerns are valid; will ask Ferenc for PHP 5.6 (put him in CC)
This is, BTW, why we should have RFC and pull request before committing
such changes.
I really thought nothing bad could happen: a few lines which look absolutely clean...

(Although it's not worth a RFC, I should have PR that first…)

Bob


Search Discussions

  • Bob Weinand at May 5, 2014 at 10:06 pm

    Am 06.05.2014 um 00:00 schrieb Hannes Magnusson <hannes.magnusson@gmail.com>:
    Bump.

    How did this end?

    -Hannes

    On Sun, Apr 13, 2014 at 1:50 PM, Bob Weinand wrote:
    Am 13.4.2014 um 22:32 schrieb Stas Malyshev <smalyshev@sugarcrm.com>:
    Hi!
    an API change. The interface hasn't changed.
    It has. Formerly, you did not have a parameter supplied for this
    function, starting with next version, you do. If you write code that
    implements this function, you need to know it changed.

    Also, this change seems to break a lot of tests, see:
    https://travis-ci.org/php/php-src/builds/22895161

    Looks like it is not as BC safe as it was intended. Please fix it or
    revert the change - BC break in 5.4 is not acceptable.
    The tests fail, because Zend checks for the number of parameters passed and warns then.
    It'd only be an API change if it forced the user to change its declaration.
    Not true. For example, we document new functions in UPGRADING, even
    though adding new functions does not force anybody to change anything.
    Same with new parameter to existing functions - see for example "Changes
    to existing functions" section.
    I thought UPGRADING would be only for new versions (not micros).
    Looking at it again, I don't think we can put this patch in either 5.4
    or 5.5. It changes extension API and breaks any module that implements
    Countable because most of them expect 0 params and will produce warning
    if given one param. Right now it broke Phar and SplObserver, but
    third-party modules will have the same issue. Unless you find a way to
    fix this in a generic way, we can only do it in 5.6, and even that only
    with permission from Ferenc, since we're in the freeze now. In any case,
    this means UPGRADING and probably UPGRADING.INTERNALS note is absolutely
    necessary, as every extension having Countable classes will need to be
    fixed.
    Btw. I really didn't think about the fact that zpp warns if there are too many
    parameters. I think your concerns are valid; will ask Ferenc for PHP 5.6 (put him in CC)
    This is, BTW, why we should have RFC and pull request before committing
    such changes.
    I really thought nothing bad could happen: a few lines which look absolutely clean...

    (Although it's not worth a RFC, I should have PR that first…)

    Bob
    It’s now PHP 5.6 upwards and there are notes on it on the right places now.

    Should be fine now I think.

    Bob
  • Michael Wallner at May 6, 2014 at 7:05 am

    On 6 May 2014 00:07, Bob Weinand wrote:
    Am 06.05.2014 um 00:00 schrieb Hannes Magnusson <hannes.magnusson@gmail.com>:
    Bump.

    How did this end?
    It’s now PHP 5.6 upwards and there are notes on it on the right places now.

    Should be fine now I think.
    I recently crashed into that one, too. But now there's a note in
    UPGRADING.INTERNALS and I think it's pretty fine.


    --
    Regards,
    Mike

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedMay 5, '14 at 10:01p
activeMay 6, '14 at 7:05a
posts3
users3
websitephp.net

People

Translate

site design / logo © 2022 Grokbase