On Tue, Oct 15, 2019 at 10:57 PM vignesh C <vignes...@gmail.com> wrote: > > On Wed, Oct 9, 2019 at 11:37 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Tue, Oct 8, 2019 at 8:19 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > > > Amit Kapila <amit.kapil...@gmail.com> writes: > > > > On Wed, Oct 2, 2019 at 2:57 PM vignesh C <vignes...@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
