Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:86662
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.160.180 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <36F7469F-7D86-43F6-ABC7-9252DF707BFC@icicle.io>
References: <971AB39D-1E20-43E8-9CF1-A7F67E3C14C3@icicle.io>
	<CAGa2bXb8CaJsJorx0HTzSgEeAGXJN7DoW5vi=m+wFiUXNw+tng@mail.gmail.com>
	<556363D3.1040902@gmail.com>
	<12C3389A-AFF5-42CA-8190-E4227309DAED@icicle.io>
	<CAFMT4NppuzY1dhPVcVhFFB-0373AkuB4-dh-mynY0p6R9T76tw@mail.gmail.com>
	<CA+9eiLszbdRS5kKU73HSvvB4zqayS8RKAioX8cJZKqreejKfyQ@mail.gmail.com>
	<016201d0a6bf$96e37dc0$c4aa7940$@belski.net>
	<0A4CC87D-2E54-4202-9AA2-22856F0B6011@icicle.io>
	<CA+9eiLtmDQNCL3YJ9ahJpvkY9YU2RpPR=aPNChi1OSamCZ+Q3w@mail.gmail.com>
	<008701d0a762$4348c320$c9da4960$@belski.net>
	<36F7469F-7D86-43F6-ABC7-9252DF707BFC@icicle.io>
Date: Mon, 15 Jun 2015 17:53:22 +0300
Message-ID: <CA+9eiLvjXPB99VxcgNZjjCPZ+eDkndx+2c-N+Uyka03uLdZYbA@mail.gmail.com>
To: Aaron Piotrowski <aaron@icicle.io>, Nikita Popov <nikita.ppv@gmail.com>
Cc: Anatol Belski <anatol.php@belski.net>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e01184d7a810d5f05188f9e56
Subject: Re: [PHP-DEV] [RFC] Throwable Interface
From: dmitry@zend.com (Dmitry Stogov)

--089e01184d7a810d5f05188f9e56
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Mon, Jun 15, 2015 at 4:55 PM, Aaron Piotrowski <aaron@icicle.io> wrote:

>
> On Jun 15, 2015, at 6:56 AM, Anatol Belski <anatol.php@belski.net> wrote:
>
> Hi Dmitry,
>
> -----Original Message-----
> From: Dmitry Stogov [mailto:dmitry@zend.com <dmitry@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 potentiall=
y
> 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 i=
s
> 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 i=
t
> doesn't cause unnecessary delays in the release process.
>
>
>
> Hi Dmitry and Anatol,
>
> I fixed three more tests that were missed when merging, as I only checked
> those
> that failed after the merge. I should have used find and replace again
> after the
> merge, which is how I changed the majority of the tests. Only a few tests
> required
> manual changes, mostly because of hard-coded string lengths, and one or t=
wo
> tests used the class name Error. Note that Nikita recently changed nearly
> all
> these tests and many others when updating the phrasing on uncaught
> exception
> messages. While many tests were changed, users won=E2=80=99t have such is=
sues
> because they aren=E2=80=99t catching these exceptions yet.
>
> Would you like me to move zend_ce_throwable to zend_exceptions.h/c or is
> that
> something you=E2=80=99d take care of after the merge?
>

I don't care about this a lot. I just think it's better.
If you don't see any problems, please, move the code.


> I am strongly against naming something _____Exception if it doesn=E2=80=
=99t extend
> Exception. This was most of the point of the RFC, as I find code such as
> `catch (Exception $e) { =E2=80=A6 } catch (EngineException) { =E2=80=A6 }=
` very
> unintuitive and I
> feel it will lead to confusion for users.
>
> I sifted through search results on GitHub looking for usage of Error befo=
re
> proposing the RFC and also found only a couple of actual uses from
> generally
> unused projects. Most of the other results are forks of an old version of
> PHPUnit.
> Namespaces have been around long enough that most libraries and apps will
> not
> have such a class in the root namespace. As far as BC breaks go, renaming=
 a
> class in an app is a fairly trivial one to make, and one that very, very
> few people
> will have to do. I feel having a simple name such as Error is more
> important than
> avoiding naming conflicts with a very small amount of code.
>

Nikita, what is your opinion?
If you don't care, I won't as well.

Thanks. Dmitry.



>
> Regards,
> Aaron Piotrowski
>
>

--089e01184d7a810d5f05188f9e56--