Re: Ordering of header file inclusion

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