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>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2019-10-22T07:26:46Z
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 Mon, Oct 21, 2019 at 11:04 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, Oct 21, 2019 at 8:47 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> >
> Thanks for the suggestions.
> Updated patch contains the fix based on Tom Lane's Suggestion.
> Let me know your thoughts for further revision if required.
>

Few comments on 0001-Ordering-of-header-files-in-contrib-dir-oct21.patch
-----------------------------------------------------------------------------------------------------------
1.
--- 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"
 #include "EAN13.h"
 #include "ISBN.h"
 #include "ISMN.h"

Why only "isn.h" is moved and not others?

2.
+++ b/contrib/pgcrypto/px-crypt.c
@@ -31,9 +31,8 @@

 #include "postgres.h"

-#include "px.h"
 #include "px-crypt.h"
-
+#include "px.h"

I think such ordering was fine.  Forex. see, hash.c (hash.h was
included first and then hash_xlog.h).

3.
+#include "_int.h"
 #include "access/gist.h"
 #include "access/stratnum.h"

-#include "_int.h"
-

Do we need to give preference to '_'?  Is it being done somewhere
else?  It is not that this is wrong, just that I am not sure about
this.

4.
--- a/contrib/hstore/hstore_io.c
+++ b/contrib/hstore/hstore_io.c
@@ -8,6 +8,7 @@
 #include "access/htup_details.h"
 #include "catalog/pg_type.h"
 #include "funcapi.h"
+#include "hstore.h"
 #include "lib/stringinfo.h"
 #include "libpq/pqformat.h"
 #include "utils/builtins.h"
@@ -18,7 +19,6 @@
 #include "utils/memutils.h"
 #include "utils/typcache.h"

-#include "hstore.h"

 PG_MODULE_MAGIC;

This created an extra white line.

5.
While reviewing, I noticed that in contrib/intarray/_int_op.c, there
is an extra white line between postgres.h and its first include.  I
think we can make that as well consistent.

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