>
> > As this is a significant change to the RFC, I'd recommend moving it back
> to
> > discussion first.
>

I have suspended the vote and will be resumed after we discuss the
deprecation notice.


> > Without some further evaluation, I am not sure whether throwing a
> > deprecation from a low level hash API function like this is safe. The
> > deprecation may be converted into an exception and it's not immediately
> > clear that calling code will handle this correctly. In any case, even
> > assuming this is safe, what is the expected behavior if the deprecation
> > notice is converted into an exception? As the implementation stands right
> > now, the element will still be inserted into the array. Is this
> intentional?
> > ("We don't care" is also a valid answer -- it's an edge case.)
>

It seems to be handled correctly. However, the current PR may still change
if there are any issues with it.


> > It would also be nice to quickly check what the performance impact of
> this
> > change is (a micro-benchmark on array appends should be enough). The
> patch
> > adds a number of additional checks in a very hot code-path, which may or
> > may not have a measurable impact.
>

Running the relevant parts of Zend/bench.php on master and the PR branch
indicates that it shouldn't have a serious on performance.


> My concern is that valid code which is working fine and will continue
> working
> fine with the change described in the rfc will now start spitting out
> notices.
>
> How often and how many? I don't know.
>

That's a very valid concern. I believe that the benefit outweighs the
potential noise but, if most people disagree, there are other options like
waiting until the last 7.x or not introducing it at all.

In this case I would like to hear some more feedback on the usefulness of
the deprecation notice. If necessary, I can open a second vote for the
deprecation notice keeping the first one only about the main point of the
RFC.

Regards,
Pedro

Reply via email to