Horton

Thread

Commits

  1. Remove unneeded includes of <sys/stat.h>

  2. Remove outdated include

  3. Straighten include order in guc-file.l

  1. Incorrect include file order in guc-file.l

    Michael Paquier <michael@paquier.xyz> — 2022-11-02T05:29:50Z

    Hi all,

While reviewing a different patch, I have noticed that guc-file.l
includes sys/stat.h in the middle of the PG internal headers.  The
usual practice is to have first postgres[_fe].h, followed by the
system headers and finally the internal headers.  That's a nit, but
all the other files do that.

{be,fe}-secure-openssl.c include some exceptions though, as documented
there.

Thoughts?
--
Michael

  2. Re: Incorrect include file order in guc-file.l

    Julien Rouhaud <rjuju123@gmail.com> — 2022-11-02T06:01:05Z

    Hi,

On Wed, Nov 02, 2022 at 02:29:50PM +0900, Michael Paquier wrote:
>
> While reviewing a different patch, I have noticed that guc-file.l
> includes sys/stat.h in the middle of the PG internal headers.  The
> usual practice is to have first postgres[_fe].h, followed by the
> system headers and finally the internal headers.  That's a nit, but
> all the other files do that.
>
> {be,fe}-secure-openssl.c include some exceptions though, as documented
> there.

Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.

  3. Re: Incorrect include file order in guc-file.l

    John Naylor <john.naylor@enterprisedb.com> — 2022-11-02T06:53:21Z

    On Wed, Nov 2, 2022 at 1:01 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> Hi,
>
> On Wed, Nov 02, 2022 at 02:29:50PM +0900, Michael Paquier wrote:
> >
> > While reviewing a different patch, I have noticed that guc-file.l
> > includes sys/stat.h in the middle of the PG internal headers.  The
> > usual practice is to have first postgres[_fe].h, followed by the
> > system headers and finally the internal headers.  That's a nit, but
> > all the other files do that.
> >
> > {be,fe}-secure-openssl.c include some exceptions though, as documented
> > there.
>
> Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.

Yeah. +1, thanks.

--
John Naylor
EDB: http://www.enterprisedb.com

  4. Re: Incorrect include file order in guc-file.l

    John Naylor <john.naylor@enterprisedb.com> — 2022-11-03T05:40:19Z

    On Wed, Nov 2, 2022 at 1:01 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> Hi,
>
> On Wed, Nov 02, 2022 at 02:29:50PM +0900, Michael Paquier wrote:
> >
> > While reviewing a different patch, I have noticed that guc-file.l
> > includes sys/stat.h in the middle of the PG internal headers.  The
> > usual practice is to have first postgres[_fe].h, followed by the
> > system headers and finally the internal headers.  That's a nit, but
> > all the other files do that.
> >
> > {be,fe}-secure-openssl.c include some exceptions though, as documented
> > there.
>
> Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.

I've pushed this, thanks!

--
John Naylor
EDB: http://www.enterprisedb.com

  5. Re: Incorrect include file order in guc-file.l

    Michael Paquier <michael@paquier.xyz> — 2022-11-03T11:40:32Z

    On Thu, Nov 03, 2022 at 12:40:19PM +0700, John Naylor wrote:
> On Wed, Nov 2, 2022 at 1:01 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>> Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.
> 
> I've pushed this, thanks!

Thanks for the commit.  I've wanted to get it done yesterday but life
took over faster than that.  Before committing the change, there is
something I have noticed though: this header does not seem to be
necessary at all and it looks that there is nothing in guc-file.l that
needs it.  Why did you add it in dac048f to begin with?
--
Michael

  6. Re: Incorrect include file order in guc-file.l

    John Naylor <john.naylor@enterprisedb.com> — 2022-11-03T12:19:07Z

    On Thu, Nov 3, 2022 at 6:40 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Nov 03, 2022 at 12:40:19PM +0700, John Naylor wrote:
> > On Wed, Nov 2, 2022 at 1:01 PM Julien Rouhaud <rjuju123@gmail.com>
wrote:
> >> Agreed, it's apparently an oversight in dac048f71eb.  +1 for the patch.
> >
> > I've pushed this, thanks!
>
> Thanks for the commit.  I've wanted to get it done yesterday but life
> took over faster than that.  Before committing the change, there is
> something I have noticed though: this header does not seem to be
> necessary at all and it looks that there is nothing in guc-file.l that
> needs it.  Why did you add it in dac048f to begin with?

Because it wouldn't compile otherwise, obviously. :-)

I must have been working on it before bfb9dfd93720

--
John Naylor
EDB: http://www.enterprisedb.com

  7. Re: Incorrect include file order in guc-file.l

    Michael Paquier <michael@paquier.xyz> — 2022-11-03T22:42:07Z

    On Thu, Nov 03, 2022 at 07:19:07PM +0700, John Naylor wrote:
> Because it wouldn't compile otherwise, obviously. :-)
> 
> I must have been working on it before bfb9dfd93720

Hehe, my fault then ;p

The CI is able to complete without it.  Would you mind if it is
removed?  If you don't want us to poke more at the bear, that's a nit
so leaving things as they are is also fine by me.
--
Michael

  8. Re: Incorrect include file order in guc-file.l

    John Naylor <john.naylor@enterprisedb.com> — 2022-11-04T00:55:20Z

    On Fri, Nov 4, 2022 at 5:42 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> The CI is able to complete without it.  Would you mind if it is
> removed?  If you don't want us to poke more at the bear, that's a nit
> so leaving things as they are is also fine by me.

I've removed it.

--
John Naylor
EDB: http://www.enterprisedb.com

  9. Re: Incorrect include file order in guc-file.l

    Michael Paquier <michael@paquier.xyz> — 2022-11-05T03:34:02Z

    On Fri, Nov 04, 2022 at 07:55:20AM +0700, John Naylor wrote:
> I've removed it.

Thanks.

Aha, there were three more of these, as of rewriteheap.c, copydir.c
and pgtz.c that I also forgot to clean up in bfb9dfd..
--
Michael