Re: Ordering of header file inclusion

On Tue, Oct 15, 2019 at 10:57 PM vignesh C <vignesh21@gmail.com> wrote:
>
> Attached patch contains the fix based on the comments suggested.
> I have added/deleted extra lines in certain places so that the
> readability is better.
>

Hmm, I am not sure if that is better in all cases.  It seems to be
making the code look inconsistent at few places.  See comments below:

1.
diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index 4b2186b..45215ba 100644
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -15,6 +15,7 @@
 #include "access/genam.h"
 #include "access/generic_xlog.h"
 #include "access/tableam.h"
+#include "bloom.h"
 #include "catalog/index.h"
 #include "miscadmin.h"
 #include "storage/bufmgr.h"
@@ -23,7 +24,6 @@
 #include "utils/memutils.h"
 #include "utils/rel.h"

-#include "bloom.h"

 PG_MODULE_MAGIC;

diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index 49e364a..4b9a2b8 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -13,14 +13,14 @@
 #include "postgres.h"

 #include "access/relscan.h"
-#include "pgstat.h"
+#include "bloom.h"
 #include "miscadmin.h"
+#include "pgstat.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"

-#include "bloom.h"

 /*
  * Begin scan of bloom index.

The above changes lead to one extra line between the last header
include and from where the actual code starts.

2.
diff --git a/contrib/intarray/_int_bool.c b/contrib/intarray/_int_bool.c
index 91e2a80..0d2f667 100644
--- a/contrib/intarray/_int_bool.c
+++ b/contrib/intarray/_int_bool.c
@@ -3,11 +3,11 @@
  */
 #include "postgres.h"

+#include "_int.h"
+
 #include "miscadmin.h"
 #include "utils/builtins.h"

-#include "_int.h"
-
 PG_FUNCTION_INFO_V1(bqarr_in);
 PG_FUNCTION_INFO_V1(bqarr_out);
 PG_FUNCTION_INFO_V1(boolop);
diff --git a/contrib/intarray/_int_gin.c b/contrib/intarray/_int_gin.c
index 7aebfec..d6241d4 100644
--- a/contrib/intarray/_int_gin.c
+++ b/contrib/intarray/_int_gin.c
@@ -3,11 +3,11 @@
  */
 #include "postgres.h"

+#include "_int.h"
+
 #include "access/gin.h"
 #include "access/stratnum.h"

Why extra line after inclusion of _int.h?

3.
diff --git a/contrib/intarray/_int_tool.c b/contrib/intarray/_int_tool.c
index fde8d15..75ad04e 100644
--- a/contrib/intarray/_int_tool.c
+++ b/contrib/intarray/_int_tool.c
@@ -5,10 +5,10 @@

 #include <limits.h>

-#include "catalog/pg_type.h"
-
 #include "_int.h"

+#include "catalog/pg_type.h"
+

Why extra lines after both includes?

4.
diff --git a/contrib/intarray/_intbig_gist.c b/contrib/intarray/_intbig_gist.c
index 2a20abe..87ea86c 100644
--- a/contrib/intarray/_intbig_gist.c
+++ b/contrib/intarray/_intbig_gist.c
@@ -3,12 +3,12 @@
  */
 #include "postgres.h"

+#include "_int.h"
+
 #include "access/gist.h"
 #include "access/stratnum.h"
 #include "port/pg_bitutils.h"

-#include "_int.h"
-
 #define GETENTRY(vec,pos) ((GISTTYPE *)
DatumGetPointer((vec)->vector[(pos)].key))
 /*
 ** _intbig methods
diff --git a/contrib/isn/isn.c b/contrib/isn/isn.c
index 0c2cac7..36bb582 100644
--- a/contrib/isn/isn.c
+++ b/contrib/isn/isn.c
@@ -15,9 +15,9 @@
 #include "postgres.h"

 #include "fmgr.h"
+#include "isn.h"
 #include "utils/builtins.h"

-#include "isn.h"

Again extra spaces.  I am not why you have extra spaces in a few cases.

I haven't reviewed it completely, but generally, the changes seem to
be fine.  Please see if you can be consistent in extra space between
includes.  Kindly check the same throughout the patch.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com