On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund <andres@anarazel.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@anarazel.de> wrote: >> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.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?
