On Thu, Nov 25, 2010 at 05:02, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
> Something like the attached, version 5 of the patch? I've been using the
> function name ParseConfigFp because the internal parameter was called fp
> in the previous function body. I suppose that could easily be changed at
> commit time if necessary.
Thanks. I'll move the patch to Ready for Committer.
Here is a list of items for committers, including only cosmetic changes.
- Comments in recovery.conf.sample needs to be adjusted. # (The quotes around the value are NOT optional, but the "="
is.)It seems to be the only description about quotes are not omissible before.
- We might not need to check the result of ParseConfigFp() because it always raises FATAL on errors.
- We could remove the unused argument "calling_file" in ParseConfigFp().
- I feel "struct name_value_pair" and "ConfigNameValuePair" a bit redundant names. I'd prefer something like
ConfigVariable.
- "fp" might be a better name than FILE *fd. There are 4 usages in xlog.c.
> Extensions will need the support for custom_variable_classes as it is
> done now, and as you say, the recovery will just error out. You have to
> clean your recovery.conf file then try again (I just tried and confirm).
>
> I personally don't see any harm to have the features in all currently
> known uses-cases, and I don't see any point in walking an extra mile
> here to remove a feature in some cases.
Sure. We will leave them.
--
Itagaki Takahiro
