Re: PR for minor bugfix in compact()
| From: | David Gebler | Date: | Sat, 08 May 2021 00:07:22 +0000 |
| Subject: | Re: PR for minor bugfix in compact() | ||
| References: | 1 2 3 4 5 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-114297@lists.php.net to get a copy of this message | ||
Great, that's been updated, build in progress (relevant tests passing
locally). I'd have preferred TypeError but appreciate there being a
consistent convention to follow. These kind of small fixes are also about
me becoming more familiar with typical conventions in the engine - some of
this stuff is just not stuff you'd find out until you stick up a PR and get
feedback from more experienced contributors so it's all good to know.
Dave
On Sat, May 8, 2021 at 12:13 AM Ben Ramsey <ramsey@php.net> wrote:
> Christian Schneider wrote on 5/7/21 02:06:
> > I agree with George that it should be an E_WARNING first and then
> changed to a TypeError in PHP 9.
> > This should be the default process for reasons given in many other
> threads about tightening type rules IMHO.
> >
> > So no, I'd prefer if this PR to be changed to E_WARNING before merging
> it.
> >
> > - Chris
> >
>
> Not a lot of chatter about this on the list, but I also agree that this
> should be an E_WARNING in 8.1 and promoted to a TypeError in PHP 9. This
> is our standard process.
>
> David, please update the PR, and we'll get it merged.
>
> Cheers,
> Ben
>
>