Re: pgsql: Introduce replication progress tracking infrastructure.

From: Andres Freund <andres(at)anarazel(dot)de>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Introduce replication progress tracking infrastructure.
Date: 2015-05-01 10:26:16
Message-ID: 20150501102616.GB22649@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On 2015-04-30 13:56:08 +0900, Michael Paquier wrote:
> On Thu, Apr 30, 2015 at 2:37 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > Introduce replication progress tracking infrastructure.
> > [...]
>
> Some comments about the docs:
> 1) "the the":
> + <entry>
> + Create a replication origin with the the passed in external
> + name, and create an internal id for it.
> + </entry>
> + </row>

Fixed.

> 2) Missing markup <type> for oid.
> + <entry>
> + Lookup replication origin by name and return the internal
> + oid. If no corresponding replication origin is found a error
> + is thrown.
> + </entry>
> + </row>

I'm not really seeing a problem in that. To me it doesn't seem helpful
to refer to types when it's not about the type. Anyway, I added it
here and a couple other places nonetheless, I don't really care that
much.

The excerpt highlighted a 'a' vs 'an' error, found a good many more
:(. Should probably have either earlier or never learned about the
actual rules behind this stuff.

> 3) Perhaps "Check that a replication has been configured in the
> current session" instead of using a question?
> + <entry>
> + Has a replication origin been configured in the current session?
> + </entry>

Hm, reads fine to me.

> 4) Missing markup <type> for oid?
> + Replication origins consist out of a name and a oid. The name, which

Same as 2).

> 5) "will persist" or "will be persistent", not "will be persist" I guess.
> + If that's done replication progress will be persist in a crash safe

I guess I rewrote that sentence once too often...

> 6) The use of "that, that" looks weird to me. There should be only one.
> + system to one other, another problem can be that, that it is hard to avoid

Yes, that sounds wrong. I'll just remove the comma and one 'that'.

Pushed the fixups.

Thanks for the check!

Andres Freund

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2015-05-01 10:26:17 pgsql: Copy editing of the replication origins patch.
Previous Message Andres Freund 2015-05-01 09:55:08 pgsql: Fix unaligned memory access in xlog parsing due to replication o