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-16T02:40:18Z
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:
>
> On Wed, Oct 9, 2019 at 11:37 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Oct 8, 2019 at 8:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Amit Kapila <amit.kapila16@gmail.com> writes:
> > > > On Wed, Oct 2, 2019 at 2:57 PM vignesh C <vignesh21@gmail.com> wrote:
> > > >> I noticed that some of the header files inclusion is not ordered as
> > > >> per the usual standard that is followed.
> > > >> The attached patch contains the fix for the order in which the header
> > > >> files are included.
> > > >> Let me know your thoughts on the same.
> > >
> > > > +1.
> > >
> > > FWIW, I'm not on board with reordering system-header inclusions.
> > > Some platforms have (had?) ordering dependencies for those, and where
> > > that's true, it's seldom alphabetical.  It's only our own headers
> > > where we can safely expect that any arbitrary order will work.
> > >
> >
> > Okay, that makes sense.  However, I noticed that ordering for
> > system-header inclusions is somewhat random.  For ex. nodeSubPlan.c,
> > datetime.c, etc. include limits.h first and then math.h whereas
> > knapsack.c, float.c includes them in reverse order.  There could be
> > more such inconsistencies and the probable reason is that we don't
> > have any specific rule, so different people decide to do it
> > differently.
> >
> > > > I think we shouldn't remove the extra line as part of the above change.
> > >
> > > I would take out the blank lines between our own #includes.
> > >
> >
> > Okay, that would be better, but doing it half-heartedly as done in
> > patch might make it worse.  So, it is better to remove blank lines
> > between our own #includes in all cases.
> >
> Attached patch contains the fix based on the comments suggested.
>

Thanks for working on this.  I will look into this in the coming few
days or during next CF.  Can you please register it for the next CF
(https://commitfest.postgresql.org/25/)?

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