On 7/22/13 12:58 PM, Amit kapila wrote:
As per discussion, updated patch contains following changes:
1. Regression tests for Alter System are removed
2. Parsed the auto file automatically after parsing postgresql.conf
3. Removed addition of include directive in postgresql.conf
4. Removed error handling for parsing errors

These changes have shrunk the diff down to 1411 lines of code.

I'd like to identify which committer might take this on at this point. In a few respects this is "Ready For Committer" now, because the committer who takes this on is going to get a strong vote on how to resolve most of the remaining fundamental issues:

-Is this the point to finally commit to the config directory approach?

-If this does set the config directory usage precedent, is the name used for that appropriate? Whoever suggested the change from "conf.d" to "config" made an error IMHO. Every example I've found of other projects doing this style of config refactoring picked either "conf.d" or a unique, no two are alike name. I'd rather not see Postgres add yet another unique one. (I think the 'postgresql' part of postgresql.auto.conf as the name of the file is redundant too--what else would be in the Postgres config directory but postgresql files?--but that's not a major issue)

This could definitely use a round of committer level review of how the GUC handling is being done now too. That part of the code seems to have settled, and things like using the new validate_conf_option could be committed even with other parts still being discussed. Exactly how to best break this out into useful commits is another decision that really needs some input from the potential committer though.

--
Greg Smith   2ndQuadrant US    g...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to