On Mon, Oct 21, 2019 at 11:04 PM vignesh C <vignes...@gmail.com> wrote: > > On Mon, Oct 21, 2019 at 8:47 AM Amit Kapila <amit.kapil...@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
