From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Aizaz Ahmed <aahmed(at)redhat(dot)com> |
Cc: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Fernando Nasser <fnasser(at)redhat(dot)com> |
Subject: | Re: fix for new SUSET GUC variables |
Date: | 2003-07-14 20:33:14 |
Message-ID: | 13892.1058214794@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
Aizaz Ahmed <aahmed(at)redhat(dot)com> writes:
> I believe when updating the GucContext enum, it is also necessary to
> update the GucContext_names [] in backend/utils/misc/help_config.c.
> The need to do this was supposed to be added as a comment to the guc.h
> file, right about where GucContext is defined, but it seems as if that
> part of the patch was not applied.
I did not include that comment because it seemed misleading (that array
is far from the only place that has to be adjusted when adding a new
GucContext value) as well as distracting (GucContext_names is not
exactly the most important thing to know about when trying to understand
GucContext).
We don't normally try to enumerate in comments all the places you'd need
to change when adding to an enum or other widely-used definition. You're
supposed to find them by searching the source code for references to the
existing values. Depending on comments for that sort of thing is far
too error-prone --- you can just about guarantee that the comment will
fail to track new uses.
The reason Bruce's patch broke the array is that he applied an old patch
without sufficient checking on what had changed in the meantime.
If the comment had been there, it would not have saved him from this
error, since I doubt it would have occurred to him to look for such a
comment.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2003-07-14 20:35:06 | Re: typo in src/include/utils/array.h |
Previous Message | Jon Jensen | 2003-07-11 16:25:48 | Revised sslmode patch |