On Tue, Oct 15, 2019 at 10:57 PM vignesh C <vignes...@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
