Re: [PATCH] Improve Configuration Checking and Parsing

On 01/05/2013 05:12 PM, Tianyin Xu wrote:

> @@ -193,6 +290,8 @@
> return false;
> } else if ((port = strtol(token, &tmp, 10)), !*tmp) {
> /* port */
> + port = xatos(token);
> +
> } else {
> host = token;
> port = 0;

Please do not set "port" twice because it is confusing and the comma
operator in the expression only makes things worse. Do something like
this instead:

    else if (strtol(token, &tmp, 10) && !*tmp) {
      port = xatos(token);
    }

> + debugs(0, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: 'enable' and 'disable' are deprecated. Please update to use value 'on' and 'off'.");

s/value/values/

or simply remove the word "value".

IMO, it would be better to make the above message specific to 'enable'
or 'disable' case, so that the admin knows which value to grep for in
the config file.

Have others discussed deprecation of enable/disable? If not, perhaps
they do not need to be deprecated at all?

Thank you,

Alex.
Received on Thu Jan 10 2013 - 16:14:43 MST