Horton

Re: Ordering of header file inclusion

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2019-10-17T11:14:41Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Make the order of the header file includes consistent.

  2. Make the order of the header file includes consistent in backend modules.

  3. Make the order of the header file includes consistent in non-backend modules.

  4. Make the order of the header file includes consistent in contrib modules.

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