On 2019-Feb-01, Dmitry Dolgov wrote:
> > On Fri, Feb 1, 2019 at 12:33 PM Alvaro Herrera
> > wrote:
> > > * Use NULL as a default value where it was an empty string before (this
> > > required few minor changes for some part of the code outside
> > > ArchiveEntry)
> >
> > I would rename the f
> On Fri, Feb 1, 2019 at 12:33 PM Alvaro Herrera
> wrote:
>
> pgindent didn't like your layout with two-space indents for the struct
> members :-( I thought it was nice, but oh well. This means we can do
> away with the newline at each callsite, and I didn't like the trailing
> comma (and I hav
On 2019-Feb-01, Alvaro Herrera wrote:
> ... so this code
>
> if (!ropt->noOwner)
> sanitized_owner = replace_line_endings(te->owner);
> else
> sanitized_owner = pg_strdup("-");
>
> can become
> sanitized_owner
pgindent didn't like your layout with two-space indents for the struct
members :-( I thought it was nice, but oh well. This means we can do
away with the newline at each callsite, and I didn't like the trailing
comma (and I have vague recollections that some old compilers might
complain about the
> On 24 Jan 2019, at 13:12, Dmitry Dolgov <9erthali...@gmail.com> wrote:
> Here is another version, where I accumulated all the suggestions:
Nothing sticks out during review and AFAICT all comments have been addressed.
Everything works as expected during (light) testing between master and an old
Here is another version, where I accumulated all the suggestions:
* Use NULL as a default value where it was an empty string before (this
required few minor changes for some part of the code outside ArchiveEntry)
* Rename defn, descr to createStmt, description
* Use a macro to avoid pgindent e
On 1/23/19 12:25 PM, Andres Freund wrote:
> On 2019-01-23 12:22:23 -0500, Chapman Flack wrote:
>> ArchiveEntry(fout, dbCatId, dbDumpId, .tag = datname, .owner = dba,
>> .desc = "DATABASE", .section = SECTION_PRE_DATA,
>> .defn = creaQry->data, .dropStmt = delQry->data);
Andres Freund writes:
> Btw, do you have an opionion on keeping catId / dumpId outside/inside
> the argument struct?
I'd go for outside, since they're not optional. Not dead set on that
though.
regards, tom lane
On 2019-01-23 12:32:06 -0500, Tom Lane wrote:
> Andres Freund writes:
> > On 2019-01-23 13:58:07 -0300, Alvaro Herrera wrote:
> >> It does. How does pgindent behave with it?
>
> > It craps out:
> > Error@3649: Unbalanced parens
> > Warning@3657: Extra )
>
> > But that can be worked around with
Andres Freund writes:
> On 2019-01-23 13:58:07 -0300, Alvaro Herrera wrote:
>> It does. How does pgindent behave with it?
> It craps out:
> Error@3649: Unbalanced parens
> Warning@3657: Extra )
> But that can be worked around with something like
> te = ArchiveEntry(fout, tdinfo->dobj.c
On 2019-01-23 12:22:23 -0500, Chapman Flack wrote:
> On 1/23/19 12:10 PM, Andres Freund wrote:
> > On 2019-01-23 12:05:10 -0500, Chapman Flack wrote:
> >> [1] https://github.com/NetBSD/src/blob/trunk/sys/sys/midiio.h#L709
>
> > I'm not really seeing this being more than obfuscation in this case. T
On 2019-01-23 13:58:07 -0300, Alvaro Herrera wrote:
> Hello
>
> On 2019-Jan-23, Andres Freund wrote:
>
> > > All the arguments, except Archive, CatalogId and DumpId I've moved
> > > into the ArchiveOpts structure. Not all of them could be empty before, but
> > > anyway it seems better for consist
On 1/23/19 12:10 PM, Andres Freund wrote:
> On 2019-01-23 12:05:10 -0500, Chapman Flack wrote:
>> [1] https://github.com/NetBSD/src/blob/trunk/sys/sys/midiio.h#L709
> I'm not really seeing this being more than obfuscation in this case. The
> only point of the macro is to set the .tag and .op eleme
Hi,
On 2019-01-23 12:05:10 -0500, Chapman Flack wrote:
> On 1/23/19 10:12 AM, Dmitry Dolgov wrote:
> > To make this discussion a bit more specific, I've created a patch of how
> > it can look like.
> A little bit of vararg-macro action can make such a design look
> even tidier, cf. [1].
> [1] htt
Hi,
On 2019-01-23 13:58:07 -0300, Alvaro Herrera wrote:
> I'd use ArchiveEntryOpts as struct name; ArchiveOpts sounds wrong.
Brevity would be of some advantage IMO, because it'll probably determine
how pgindent indents the arguments, because the struct name will be in
the arguments.
> Also, the
Chapman Flack writes:
> Or are compilers without vararg macros still in the supported mix?
No.
regards, tom lane
On 1/23/19 10:12 AM, Dmitry Dolgov wrote:
> To make this discussion a bit more specific, I've created a patch of how
> it can look like.
A little bit of vararg-macro action can make such a design look
even tidier, cf. [1].
Or are compilers without vararg macros still in the supported mix?
-Chap
Hello
On 2019-Jan-23, Andres Freund wrote:
> > All the arguments, except Archive, CatalogId and DumpId I've moved
> > into the ArchiveOpts structure. Not all of them could be empty before, but
> > anyway it seems better for consistency and readability. Some of the
> > arguments
> > had empty str
Hi,
On 2019-01-23 16:12:15 +0100, Dmitry Dolgov wrote:
> To make this discussion a bit more specific, I've created a patch of how it
> can
> look like.
Thanks.
> All the arguments, except Archive, CatalogId and DumpId I've moved
> into the ArchiveOpts structure. Not all of them could be empty b
> On 2019-01-17 09:29:04 -0800, Andres Freund wrote:
> On 2019-01-17 10:23:39 -0500, Tom Lane wrote:
> > I don't buy the argument that this would move the goalposts in terms
> > of how much work it is to add a new argument. You'd still end up
> > touching every call site.
>
> Why? A lot of argumen
On Wed, 16 Jan 2019 at 17:45, Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> Hi,
>
> During the discussion in [1] an idea about refactoring ArchiveEntry was
> suggested. The reason is that currently this function has significant number
> of
> arguments that are "optional", and every change that
Hi,
On 2019-01-17 09:29:04 -0800, Andres Freund wrote:
> On 2019-01-17 10:23:39 -0500, Tom Lane wrote:
> > I don't buy the argument that this would move the goalposts in terms
> > of how much work it is to add a new argument. You'd still end up
> > touching every call site.
>
> Why? A lot of arg
Greetings,
* Andres Freund (and...@anarazel.de) wrote:
> On 2019-01-17 10:23:39 -0500, Tom Lane wrote:
> > Alvaro Herrera writes:
> > > On 2019-Jan-16, Dmitry Dolgov wrote:
> > >> ArchiveEntry((ArchiveArgs){.tablespace = 3,
> > >> .dumpFn = somefunc,
> > >> ...});
> >
> > > Is there real savings
On 2019-01-17 10:23:39 -0500, Tom Lane wrote:
> Alvaro Herrera writes:
> > On 2019-Jan-16, Dmitry Dolgov wrote:
> >> ArchiveEntry((ArchiveArgs){.tablespace = 3,
> >> .dumpFn = somefunc,
> >> ...});
>
> > Is there real savings to be had by doing this? What would be the
> > arguments to each functi
Alvaro Herrera writes:
> On 2019-Jan-16, Dmitry Dolgov wrote:
>> ArchiveEntry((ArchiveArgs){.tablespace = 3,
>> .dumpFn = somefunc,
>> ...});
> Is there real savings to be had by doing this? What would be the
> arguments to each function? Off-hand, I'm not liking this idea too
> much.
I'm not
On 2019-Jan-16, Dmitry Dolgov wrote:
> Proposed idea is to refactor out all/optional arguments into a separate data
> structure, so that adding/removing a new argument wouldn't change that much of
> unrelated code. Then for every invocation of ArchiveEntry this structure needs
> to be prepared be
> On Wed, Jan 16, 2019 at 1:16 PM Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> Hi,
>
> During the discussion in [1] an idea about refactoring ArchiveEntry was
> suggested. The reason is that currently this function has significant number
> of
> arguments that are "optional", and every change t
Hi,
During the discussion in [1] an idea about refactoring ArchiveEntry was
suggested. The reason is that currently this function has significant number of
arguments that are "optional", and every change that has to deal with it
introduces a lot of useless diffs. In the thread, mentioned above, su
28 matches
Mail list logo
