On 4/27/15 10:31 AM, Sawada Masahiko wrote:
> Thank you for your review comment!
> The latest patch is attached.

Looks good overall - a few more comments below:

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
+    <row>
+     <entry><structfield>seqno</structfield></entry>
+     <entry><structfield>integer</structfield></entry>
+     <entry>Sequence number of current view</entry>

I would suggest:

Order in which the setting was loaded from the configuration

+    </row>
+    <row>
+     <entry><structfield>name</structfield></entry>
+     <entry><structfield>text</structfield></entry>

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
+ * show_all_file_settings
+ */

It would be good to get more detail in the function comment, as well as
more comments in the function itself.

A minor thing, but there are a number of whitespace errors when applying
the patch:

../000_pg_file_settings_view_v6.patch:159: indent with spaces.
../000_pg_file_settings_view_v6.patch:160: indent with spaces.
../000_pg_file_settings_view_v6.patch:161: indent with spaces.
../000_pg_file_settings_view_v6.patch:178: indent with spaces.
            guc_array->name = guc_strdup(FATAL, item->name);
../000_pg_file_settings_view_v6.patch:179: indent with spaces.
            guc_array->value = guc_strdup(FATAL, item->value);
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.

I'm sure the committer would appreciate it if you'd clean those up.

- David Steele

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to