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
27 matches
Mail list logo
