2009/7/8 Ilia Alshanetsky <i...@prohost.org>:
> All of the identified issues can be resolved and none of them represent a
> major challenge to address. However, if there is no consensus to put this in
OK, but you had not said you would resolve them. I would appreciate
some detail on what you will do to address them.
> the near future (which at this point is 5.3), I have hard time justifying
> spending further time on this. The original patch that was posted, that did
> break BC was far simpler and featureless, the changes since (which took a
> fair amount of work) were specifically made to address some of the main
> concerns that were on the list. I feel what is on the table right now is
> pretty close to what a final product could be, to have a vote on it. If
Personally, I had gotten the impression that this was the final
product, and that we could take it or leave it.
> decision is made to proceed within a practical release schedule, then
> suffice to say that I'd be more then happy to put further time to address
> the minor issues indicated.
I recommend:
- stop the vote
- address the issues (possibly deferring them until after voting, or whatever)
- write an RFC
- wait for Lukas to finish what he's doing
- new vote, more options (5.3.x/5.4/6.0, Lukas'/yours, make it clear
what we're voting for)
Thanks,
Paul
> On 7-Jul-09, at 7:07 PM, Paul Biggar wrote:
>
>> 2009/7/7 Johannes Schlüter <johan...@php.net>:
>>>
>>> On Mon, 2009-07-06 at 20:52 -0400, Ilia Alshanetsky wrote:
>>>>
>>>> Last week or so there was a fairly detailed discussion on the
>>>> internals list regarding type hinting based on my original patch.
>>
>>
>>> Having an "old" 5.3 extension with a typehint expecting an array
>>> arg_info.array_type_hint will be set to 1.
>>> The newly compiled engine with this patch will then do
>>>
>>> + /* existing type already matches the hint or forced type
>>> */
>>> + if (Z_TYPE_P(arg) == cur_arg_info->array_type_hint ||
>>> Z_TYPE_P(arg) == (cur_arg_info->array_type_hint ^ (1<<7))) {
>>>
>>> as it's main type check, but Z_TYPE_P(arg) will be IS_ARRAY (5) which
>>> doesn't match the 1 provided by the old extension, other checks in there
>>> will fail too, so the valid param will be rejected whereas an integer
>>> (IS_LONG 1) will be accepted where the extension expects an array.
>>
>> I raised this in my review, to which Ilia replied "It should be fine"
>> (http://news.php.net/php.internals/44707). I would not have thought it
>> would be fine.
>>
>> I had been thinking that Ilia would have to hack it to make 1 mean
>> array in this case, which would be ugly, but workable. Based on the
>> arguments in this thread, I believe it shouldn't go into 5.3 at all.
>> Are we allowed break the ABI for 5.4 (I would think so, but amn't
>> sure).
>>
>>
>>
>> Overall, I'm very disappointed with the way this has been conducted.
>> When reviews were posted they are not replied to (Stas posted
>> http://news.php.net/php.internals/44710, I posted
>> http://news.php.net/php.internals/44706, and I dont see any replies
>> except a cursory response to mine). Furthermore:
>> - the RFC process has been wilfully ignored (despite multiple requests)
>> - a vote was asked for when Lukas was still trying to discuss his
>> proposal
>> - the vote was take it or leave it
>> - there has been a general attitude of "throwing the toys out of the
>> pram"
>>
>>
>> I am mostly for the patch, and I 100% support the idea. However, I
>> feel I have to vote against it, and urge others to do the same, until
>> the entire mess is rectified.
>>
>> Ilia, I respect the work you have put into this, but I would ask you
>> to withdraw the patch and the vote until these things have been sorted
>> out.
>>
>>
>> -1
>>
>> Thanks,
>> Paul
>>
>>
>>
>> --
>> Paul Biggar
>> paul.big...@gmail.com
>
>
--
Paul Biggar
paul.big...@gmail.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
