On Tue, Oct 22, 2019 at 12:56 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > 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? > Fixed. Order is based on ascii table. Upper case letters will come before lower case letters. > 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). > Order is based on ascii table. Ascii value for "." is 46, Ascii value for "-" is 45. Hence have placed like: #include "px-crypt.h" #include "px.h" Not make any changes for this. If still required I can modify. > 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. > The changes are done based on ascii table. Ascii value of "_" is 95. Ascii value of "a" is 97. Hence _int.h is place before access/gist.h. I have not made any changes for this. If still required I can modify. > 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. > Fixed. > 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. > Fixed. Thanks for the comments. Attached patch has the updated changes.
Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
0001-Ordering-of-header-files-in-contrib-dir-oct22.patch
Description: Binary data
