Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)

From: Ryan Murphy <ryanfmurphy(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)
Date: 2016-08-18 20:35:24
Message-ID: CAHeEsBeX4XSXW3fA5HkbHVufHAGfV1rtE34hyn+U+TYRw_sV3A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2016-08-18 16:11:27 -0400, Robert Haas wrote:
> >> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund <andres(at)anarazel(dot)de>
> wrote:
> >> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com> wrote:
> >> >
> >> >>+ { /* pg_ctl command w path, properly quoted */
> >> >>+ PQExpBuffer pg_ctl_path = createPQExpBuffer();
> >> >>+ printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
> >> >>+ bin_dir,
> >> >>+ (strlen(bin_dir) > 0) ? DIR_SEP : ""
> >> >>+ );
> >> >>+ appendShellString(start_db_cmd, pg_ctl_path->data);
> >> >>+ destroyPQExpBuffer(pg_ctl_path);
> >> >>+ }
> >> >>
> >> >>This is not really project-style to have an independent block. Usually
> >> >>those are controlled by for, while or if.
> >> >
> >> > Besides the comment positioning I'd not say that that is against the
> usual style, there's a number of such blocks already. Don't think it's
> necessarily needed here though...
> >>
> >> Really? I'd remove such blocks before committing anything, or ask for
> >> them to be removed, unless there were some special reason for having
> >> them.
> >
> > Well, reducing the scope of variables *can* be such a reason, no? As I
> > said, I don't see any reason here, but in general, it's imo a reasonable
> > tool on one's belt.
>
> I think it's worth reducing the scope of variables when that's as
> simple as putting them inside a block that you have to create anyway,
> but I'm skeptical about the idea that one would create a block just to
> reduce the scope of the variables. I don't think that's our usual
> practice, and I would expect the compiler to detect where the variable
> is referenced first and last anyway.
>
>
I'm can change my patch to take out that block.

I enjoy adding the blocks for explicit variable scoping and for quick
navigation in vim (the % key navigates between matching {}'s). But I want
to fit in with the style conventions of the project.

Before I change and resubmit my patch, are there any other changes, style
or otherwise, that you all would recommend?

> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-08-18 20:35:51 Re: Fix comment in ATExecValidateConstraint
Previous Message Tom Lane 2016-08-18 20:30:52 Re: WIP: About CMake v2