Everything

php

Re: Changelog / upgrading notes for promoted warnings

php.internals

Máté Kocsis

6 years ago

> Hi all,
>
> Earlier, I was writing an application-specific wrapper around the
> password_hash function, and was surprised that it could return false,
> rather than throwing an error.
>
> I then found this commit to master making it throw errors for PHP 8.0:
> https://github.com/php/php-src/commit/37c11714
>
> That's great! But ... it is a breaking change, and I can't see any note
> in UPGRADING. Is there a running list of the Warnings that have been
> promoted to Errors anywhere, and if not, should we create one before we
> forget?
>
> Regards,
>
> --
> Rowan Tommins (né Collins)
> [IMSoP]

Hi Rowan,

You are completely right, these should be (should have been) noted
somewhere.

Although, the only problem I have with including every promoted warning in
the upgrading notes
is that doing so would possibly cannibalise it as there are dozens (if not
hundreds) of
promotions that have already taken place...

Nevertheless, I'm happy to help collecting the converted warnings and
putting the list in either
the upgrading notes or any other place - whatever is going to be decided.

Máté

Christoph Becker

6 years ago

On 20.12.2019 at 00:05, Máté Kocsis wrote:

> Although, the only problem I have with including every promoted warning in
> the upgrading notes
> is that doing so would possibly cannibalise it as there are dozens (if not
> hundreds) of
> promotions that have already taken place...
>
> Nevertheless, I'm happy to help collecting the converted warnings and
> putting the list in either
> the upgrading notes or any other place - whatever is going to be decided.

Maybe a sensible compromise would be to assemble a detailed list of
these changes, and to hand it over to the doc team at some point, so
that the manual pages could (hopefully) be updated in time just before
PHP 8.0.0 will be released.  Then users could check the manual's
changelog[1] regarding the details.

[1] <https://www.php.net/manual/en/doc.changelog.php>

--
Christoph M. Becker

Rowan Collins

6 years ago

On Fri, 20 Dec 2019 at 08:37, Christoph M. Becker <cmbecker69@gmx.de> wrote:

> Maybe a sensible compromise would be to assemble a detailed list of
> these changes, and to hand it over to the doc team at some point, so
> that the manual pages could (hopefully) be updated in time just before
> PHP 8.0.0 will be released.  Then users could check the manual's
> changelog[1] regarding the details.
>

Yes, that sounds reasonable. In many cases, the individual function listing
should also have a note added at the same time; and at some future date,
some will be able to lose their "may return false" notes.


On Fri, 20 Dec 2019 at 00:34, G. P. B. <george.banyard@gmail.com> wrote:

> Moreover, I'm not sure it's helpful to have a massive list of functions

where some warnings have been promoted and others not. But this is only my
opinion

I think this makes it even *more* important to collect the list of changes.
In the case of password_hash, my understanding is that it will now never
return false; in other cases, it may still be necessary to check the return
value. The documentation should clearly state which is the case for each
function, so we need to keep track of which functions are fully-promoted
and which partially-promoted.


Regards,

--
Rowan Tommins
[IMSoP]

Peter Cowburn

6 years ago

On Fri, 20 Dec 2019 at 09:31, Rowan Tommins <rowan.collins@gmail.com> wrote:

> On Fri, 20 Dec 2019 at 08:37, Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
>
> > Maybe a sensible compromise would be to assemble a detailed list of
> > these changes, and to hand it over to the doc team at some point, so
> > that the manual pages could (hopefully) be updated in time just before
> > PHP 8.0.0 will be released.  Then users could check the manual's
> > changelog[1] regarding the details.
> >
>
>
> Yes, that sounds reasonable. In many cases, the individual function listing
> should also have a note added at the same time; and at some future date,
> some will be able to lose their "may return false" notes.

This is usually where the UPGRADING document comes in to play. While
watching all of the PRs against master I've very much not been looking
forward
to making sense of all of changes when it comes to updating the PHP manual
to
reflect them.

Christoph Becker

6 years ago

On 20.12.2019 at 10:39, Peter Cowburn wrote:

> On Fri, 20 Dec 2019 at 09:31, Rowan Tommins <rowan.collins@gmail.com> wrote:
>
>> On Fri, 20 Dec 2019 at 08:37, Christoph M. Becker <cmbecker69@gmx.de>
>> wrote:
>>
>>> Maybe a sensible compromise would be to assemble a detailed list of
>>> these changes, and to hand it over to the doc team at some point, so
>>> that the manual pages could (hopefully) be updated in time just before
>>> PHP 8.0.0 will be released.  Then users could check the manual's
>>> changelog[1] regarding the details.
>>
>> Yes, that sounds reasonable. In many cases, the individual function listing
>> should also have a note added at the same time; and at some future date,
>> some will be able to lose their "may return false" notes.
>
> This is usually where the UPGRADING document comes in to play. While
> watching all of the PRs against master I've very much not been looking
> forward
> to making sense of all of changes when it comes to updating the PHP manual
> to
> reflect them.

Then I would like to suggest a separate section in UPGRADING for these
"mass" changes, where we can note that arguments of wrong type now throw
a TypeError (instead of raising E_WARNING before), and that also some
further argument checks will have been elevated from E_WARNING to Error,
followed by a detailed list.  For instance, for ext/calendar:

- Calendar:
. cal_info(), cal_days_in_month(), cal_to_jd() and cal_from_jd() now
throw ValueError if an invalid calendar ID is given
. cal_days_in_month() now throws ValueError if an invalid date is
given
. jdtojewish() now throws ValueError if the given year is out of range

Thoughts?

--
Christoph M. Becker