Hi Dmitry, > -----Original Message----- > From: Dmitry Stogov [mailto:dmi...@zend.com] > Sent: Monday, June 15, 2015 10:53 AM > To: Aaron Piotrowski > Cc: Anatol Belski; PHP Internals > Subject: Re: [PHP-DEV] [RFC] Throwable Interface > > Hi, > > I made a quick code review, and I don't see any technical problems in > implementation. > > 1) Anyway, I think it's a bad idea to rename "EngineException" (and others) > into > "Error"(s). > This will prevent using class "Error" in applications, and may potentially > break > some of them. > I also don't like renaming in ~440 tests (I didn't review all these changes). > > 2) I think it's better to move "zend_ce_throwable" > definition/initialization from zend_interfaces.h/c into zend_exception.h/c. > At least, this will allow arg_info reuse. (it's missed now, but should be > added > now or in the future). > > 3) In ext/simplexml/tests/SimpleXMLElement_xpath_3.phpt EngineExcpetion is > not renamed into Error. > Thanks for the review. I've also tested the branch which has today's master merged in, by now it doesn't show any functional regression.
Actually I part your first concern about Error. Spent some time phishing for "class Error" on github and found three apps doing this without namespace. But that's from 100 search result pages. And those three apps are forked zero to 1 times, so pretty much low usage. This will likely break more in the world, we hardly know. The current RFC can't be changed while in voting. But how it looks like, the principle of the Trowable RFC is something everyone agrees on. Maybe do another RFC in parallel for better names? Still I'd stand for taking it into alpha2 - if we want to have it, better to arrange it in a way that it doesn't cause unnecessary delays in the release process. Regards Anatol -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
