Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:53739
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.210.42 as permitted sender)
MIME-Version: 1.0
Sender: giovanni.g@gmail.com
In-Reply-To: <4E10D453.9060606@daylessday.org>
References: <CAML1Pf7z1xYL1FGxv9Lb0pq3B=oy0DDSKE3nLQgu2zn-q-Ce-Q@mail.gmail.com>
	<4E10D453.9060606@daylessday.org>
Date: Mon, 4 Jul 2011 01:26:17 +0200
Message-ID: <CAML1Pf4L_1ug8KiBwR5TcGH6DY1Fn1_8RCkYEvVwpkEusoZOtw@mail.gmail.com>
To: Antony Dovgal <tony@daylessday.org>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Big patch for FPM (config and initialization)
From: giovanni@giacobbi.net (Giovanni Giacobbi)

On Sun, Jul 3, 2011 at 22:42, Antony Dovgal <tony@daylessday.org> wrote:
> On 07/03/2011 04:50 AM, Giovanni Giacobbi wrote:
>>
>> Detailed changelog of the patch:
>> - Renamed options "pm.status_path", "ping.path", "ping.response" into
>> a new logical group "diagnostics", so they are now respectively:
>> =A0 =A0 "diagnostics.status_path", "diagnostics.ping_path",
>> "diagnostics.ping_response".
>
> I'm not quite sure renaming config options would help anybody, in my opin=
ion
> it would do more harm than good.
> I personally would certainly expect my existing config files to continue =
to
> work after upgrade to 5_4.
> Yes, we're changing '3' to '4' in the version number, but that doesn't me=
an
> we can just go ahead and break the config file.
>

In fact I don't break them, probably I should've specified it more
clearly, but the previous names are still well accepted:
+	/* Backward compatibility options, to be removed in the next major releas=
e */
+	{ "ping.path",                          &fpm_conf_set_string,
WPO(diag_ping_path) },
+	{ "ping.response",                      &fpm_conf_set_string,
WPO(diag_ping_response) },
+	{ "pm.status_path",                     &fpm_conf_set_string,
WPO(diag_status_path) },
+

I can also add a deprecated warning at boot time if you wish. Anyway,
the patch is too big, I admit, if you don't want to accept the change
name I can create another patch without it. But the first time I read
the config file I had a strong feeling that pm.status_path and
ping_path were related that's why i set them to:
/somesecretname/fpm-status
/somesecretname/fpm-ping
Aren't you doing something like that too?

About the testing, well it works, but I didn't test it thoroughly.
Actually I already found a couple of bugs. Who is the official
maintainer of the FPM? If he is OK to merge it then I will do a very
careful test and I will also write some test cases for the config file
parsing (not sure if there is a test suite for that already, didn't
check carefully).

Thank you for your positive feedback :)

--=20
Giovanni Giacobbi