Re: Big patch for FPM (config and initialization)

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: >>     "diagnostics.status_path", "diagnostics.ping_path", >> "diagnostics.ping_response". > > I'm not quite sure renaming config options would help anybody, in my opinion > 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 mean > 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 release */ + { "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 :) -- Giovanni Giacobbi