Horton

Thread

Commits

  1. doc: Missing markup, punctuation and wordsmithing

  1. Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-09-05T09:33:49Z

    Dear all,

I have prepared a patch containing some minor inconsistencies in the 
documentation. Please, take a look.

The inconsistencies were noticed by: Ekaterina Kiryanova, Elena 
Indrupskaya, Maxim Yablokov, Anna Uraskova, Elena Karavaeva, and me.

We will be looking forward to your feedback.

The patch shall be applied to the REL_17_STABLE branch.

--
Regards,
Oleg Sibiryakov
Technical Writer
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

  2. Re: Documentation improvement patch

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-06T13:20:29Z

    > On 5 Sep 2024, at 11:33, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
> 
> Dear all,
> I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.
> The inconsistencies were noticed by: Ekaterina Kiryanova, Elena Indrupskaya, Maxim Yablokov, Anna Uraskova, Elena Karavaeva, and me. 
> We will be looking forward to your feedback.
> The patch shall be applied to the REL_17_STABLE branch.

Most of these seem fine, but I need another read-through to digest them fully.
Just a few small comments:

-        Specifies the builtin provider locale for the database default
-        collation order and character classification, overriding the setting
-        <xref linkend="create-database-locale"/>.  The <link
+        Specifies the <literal>builtin</literal> provider locale for the database
+        default collation order and character classification, overriding the
+        setting <xref linkend="create-database-locale"/>.  The <link
and
-        Specifies the locale name when the builtin provider is used. Locale support
-        is described in <xref linkend="locale"/>.
+        Specifies the locale name when the <literal>builtin</literal> provider
+        is used. Locale support is described in <xref linkend="locale"/>.


I don't think this use of "builtin" refers to the config value but rather the
type of locale, so I think it's correct to not use <literal> here.


-    for not-null constraints at all, so they are not
+    for <literal>NOT NULL</literal> constraints at all, so they are not

This seems mostly to be a question of taste, I don't think not-null is
incorrect here.

--
Daniel Gustafsson

  3. Re: Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-09-10T11:46:23Z

    Thank you for your feedback.

1. Since we do not want to use <literal> here, I suggest we hyphenate it 
as "built-in". What's your take on it?
2. Leaving not-null is fine.

--
Oleg Sibiryakov

On 06.09.2024 16:20, Daniel Gustafsson wrote:
>> On 5 Sep 2024, at 11:33, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
>>
>> Dear all,
>> I have prepared a patch containing some minor inconsistencies in the documentation. Please, take a look.
>> The inconsistencies were noticed by: Ekaterina Kiryanova, Elena Indrupskaya, Maxim Yablokov, Anna Uraskova, Elena Karavaeva, and me.
>> We will be looking forward to your feedback.
>> The patch shall be applied to the REL_17_STABLE branch.
> Most of these seem fine, but I need another read-through to digest them fully.
> Just a few small comments:
>
> -        Specifies the builtin provider locale for the database default
> -        collation order and character classification, overriding the setting
> -        <xref linkend="create-database-locale"/>.  The <link
> +        Specifies the <literal>builtin</literal> provider locale for the database
> +        default collation order and character classification, overriding the
> +        setting <xref linkend="create-database-locale"/>.  The <link
> and
> -        Specifies the locale name when the builtin provider is used. Locale support
> -        is described in <xref linkend="locale"/>.
> +        Specifies the locale name when the <literal>builtin</literal> provider
> +        is used. Locale support is described in <xref linkend="locale"/>.
>
>
> I don't think this use of "builtin" refers to the config value but rather the
> type of locale, so I think it's correct to not use <literal> here.
>
>
> -    for not-null constraints at all, so they are not
> +    for <literal>NOT NULL</literal> constraints at all, so they are not
>
> This seems mostly to be a question of taste, I don't think not-null is
> incorrect here.
>
> --
> Daniel Gustafsson
>
>
>

  4. Re: Documentation improvement patch

    Daniel Gustafsson <daniel@yesql.se> — 2024-09-10T13:02:52Z

    > On 10 Sep 2024, at 13:46, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:

> 1. Since we do not want to use <literal> here, I suggest we hyphenate it as "built-in". What's your take on it?

I think that's the right choice given the hyphenation used in the rest of the
docs.  There are a few more places on that same page which should be built-in
rather than builtin to separate the concept from the parameter value.

--
Daniel Gustafsson

  5. Re: Documentation improvement patch

    Peter Eisentraut <peter@eisentraut.org> — 2024-09-11T09:53:13Z

    On 10.09.24 15:02, Daniel Gustafsson wrote:
>> On 10 Sep 2024, at 13:46, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
> 
>> 1. Since we do not want to use <literal> here, I suggest we hyphenate it as "built-in". What's your take on it?
> 
> I think that's the right choice given the hyphenation used in the rest of the
> docs.  There are a few more places on that same page which should be built-in
> rather than builtin to separate the concept from the parameter value.

I suspect that this would lead to the opposite confusion, people 
complaining that the provider is called "builtin" not "built-in".

Arguably, the other providers are also "built in".  There are no 
user-pluggable providers at this time.

  6. Re: Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-09-13T10:50:14Z

    Here is a patch without the builtin/built-in corrections (find attached).

But I still believe the issue should be discussed further.
We actually have two options: it is either a spelling mistake (since 
built-in should written with a hyphen), or we miss the <literal> tag 
(since it is actually also a value).

So I do think we cannot really leave it as is.

-- 
Oleg Sibiryakov

On 11.09.2024 12:53, Peter Eisentraut wrote:
> On 10.09.24 15:02, Daniel Gustafsson wrote:
>>> On 10 Sep 2024, at 13:46, Oleg Sibiryakov 
>>> <o.sibiryakov@postgrespro.ru> wrote:
>>
>>> 1. Since we do not want to use <literal> here, I suggest we 
>>> hyphenate it as "built-in". What's your take on it?
>>
>> I think that's the right choice given the hyphenation used in the 
>> rest of the
>> docs.  There are a few more places on that same page which should be 
>> built-in
>> rather than builtin to separate the concept from the parameter value.
>
> I suspect that this would lead to the opposite confusion, people 
> complaining that the provider is called "builtin" not "built-in".
>
> Arguably, the other providers are also "built in".  There are no 
> user-pluggable providers at this time.
>
>
>

  7. Re: Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-10-01T08:04:36Z

    Dear all,

Here is a kind reminder of a small documentation improvement patch, 
which we started discussing a month ago.

I removed all the controversial points touched upon in this thread. 
Please, take a look once again at your convenience.

The patch shall be applied to the master branch this time.

--
Regards,
Oleg Sibiryakov
Technical Writer
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

On 13.09.2024 13:50, Oleg Sibiryakov wrote:
> Here is a patch without the builtin/built-in corrections (find attached).
>
> But I still believe the issue should be discussed further.
> We actually have two options: it is either a spelling mistake (since 
> built-in should written with a hyphen), or we miss the <literal> tag 
> (since it is actually also a value).
>
> So I do think we cannot really leave it as is.
>

  8. Re: Documentation improvement patch

    Daniel Gustafsson <daniel@yesql.se> — 2024-10-01T08:59:14Z

    > On 1 Oct 2024, at 10:04, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:

> Here is a kind reminder of a small documentation improvement patch, which we started discussing a month ago.
> 
> I removed all the controversial points touched upon in this thread. Please, take a look once again at your convenience.

In general, when submitting a docs patch it's better to not reflow the
paragraphs when a modified line becomes too long.  Reading a 4 line diff where
only one thing changed in the first becomes harder than reading a single line
diff where the line is long.  The committer can ensure the lines are reflowed
prior to a commit, or it can be left as the final revision of a patch
submission once all changes are discussed-

A few comments on this version of the patch:


-       ICU<indexterm><primary>ICU</primary></indexterm>
+       <indexterm><primary>ICU</primary></indexterm>

I don't think removing the name of the library changing the sentence from "The
icu provider uses the external ICU library" to "The icu provider uses the
external library" is an improvement.


-   by sequences.  (See <xref linkend="sql-createsequence"/>.)  The properties
+   by sequences (see <xref linkend="sql-createsequence"/>).  The properties

This is a common construction in our docs, if it's considered to be a bad
practice the case should be argued (separately) for removing all of them
instead.


-       Comma separated list of publication names for which to subscribe
+       Comma-separated list of publication names for which to subscribe

There are two more cases of "comma separated" (config.sgml and copy.sgml),
should they be changed too?


-          the failover if required, enable the subscription, and refresh the
-          subscription. See
+          the <literal>failover</literal> if required, enable the subscription,
+          and refresh the subscription. See

This refers to the act of failing over, not the property value failover, and
should not be in <literal>.


-    for not-null constraints at all, so they are not
+    for <literal>NOT NULL</literal> constraints at all, so they are not

I'm still not convinced that this change makes the documentation more readable.


-       the <command>MERGE</command> command will perform a <literal>FULL</literal>
-       join between <replaceable class="parameter">data_source</replaceable>
-       and the target table.  For this to work, at least one
+       the <command>MERGE</command> command will perform a
+       <literal>FULL JOIN</literal> between
+       <replaceable class="parameter">data_source</replaceable> and the target
+       table. For this to work, at least one

This paragraph discuss various join types, keeping it lowercase "join" matches
the remainder of the paragraph and makes it more readable IMHO.  It's not
discussing syntax the user is expected to type so need to make it so.

--
Daniel Gustafsson

  9. Re: Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-10-02T08:09:49Z

    Thank you for your kind feedback! I will take due note of the comments 
in the next documentation patches as well.

I have made all the changes as per your feedback and also corrected 
paragraph reflow.

The third version of the patch is attached for your consideration.

--
Oleg Sibiryakov

On 01.10.2024 11:59, Daniel Gustafsson wrote:
>> On 1 Oct 2024, at 10:04, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
>> Here is a kind reminder of a small documentation improvement patch, which we started discussing a month ago.
>>
>> I removed all the controversial points touched upon in this thread. Please, take a look once again at your convenience.
> In general, when submitting a docs patch it's better to not reflow the
> paragraphs when a modified line becomes too long.  Reading a 4 line diff where
> only one thing changed in the first becomes harder than reading a single line
> diff where the line is long.  The committer can ensure the lines are reflowed
> prior to a commit, or it can be left as the final revision of a patch
> submission once all changes are discussed-
>
> A few comments on this version of the patch:
>
>
> -       ICU<indexterm><primary>ICU</primary></indexterm>
> +       <indexterm><primary>ICU</primary></indexterm>
>
> I don't think removing the name of the library changing the sentence from "The
> icu provider uses the external ICU library" to "The icu provider uses the
> external library" is an improvement.
>
>
> -   by sequences.  (See <xref linkend="sql-createsequence"/>.)  The properties
> +   by sequences (see <xref linkend="sql-createsequence"/>).  The properties
>
> This is a common construction in our docs, if it's considered to be a bad
> practice the case should be argued (separately) for removing all of them
> instead.
>
>
> -       Comma separated list of publication names for which to subscribe
> +       Comma-separated list of publication names for which to subscribe
>
> There are two more cases of "comma separated" (config.sgml and copy.sgml),
> should they be changed too?
>
>
> -          the failover if required, enable the subscription, and refresh the
> -          subscription. See
> +          the <literal>failover</literal> if required, enable the subscription,
> +          and refresh the subscription. See
>
> This refers to the act of failing over, not the property value failover, and
> should not be in <literal>.
>
>
> -    for not-null constraints at all, so they are not
> +    for <literal>NOT NULL</literal> constraints at all, so they are not
>
> I'm still not convinced that this change makes the documentation more readable.
>
>
> -       the <command>MERGE</command> command will perform a <literal>FULL</literal>
> -       join between <replaceable class="parameter">data_source</replaceable>
> -       and the target table.  For this to work, at least one
> +       the <command>MERGE</command> command will perform a
> +       <literal>FULL JOIN</literal> between
> +       <replaceable class="parameter">data_source</replaceable> and the target
> +       table. For this to work, at least one
>
> This paragraph discuss various join types, keeping it lowercase "join" matches
> the remainder of the paragraph and makes it more readable IMHO.  It's not
> discussing syntax the user is expected to type so need to make it so.
>
> --
> Daniel Gustafsson
>

  10. Re: Documentation improvement patch

    Daniel Gustafsson <daniel@yesql.se> — 2024-10-02T12:58:30Z

    > On 2 Oct 2024, at 10:09, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
> 
> Thank you for your kind feedback! I will take due note of the comments in the next documentation patches as well.
> 
> I have made all the changes as per your feedback and also corrected paragraph reflow.
> 
> The third version of the patch is attached for your consideration.

Thanks, I have gone over and applied most of these changes.  I did leave out a
few (like the libc one) where the current page had multiple different versions.

--
Daniel Gustafsson

  11. Re: Documentation improvement patch

    Oleg <o.sibiryakov@postgrespro.ru> — 2024-10-07T08:54:40Z

    Thank you, Daniel.

--
Oleg Sibiryakov

On 02.10.2024 15:58, Daniel Gustafsson wrote:
>> On 2 Oct 2024, at 10:09, Oleg Sibiryakov <o.sibiryakov@postgrespro.ru> wrote:
>>
>> Thank you for your kind feedback! I will take due note of the comments in the next documentation patches as well.
>>
>> I have made all the changes as per your feedback and also corrected paragraph reflow.
>>
>> The third version of the patch is attached for your consideration.
> Thanks, I have gone over and applied most of these changes.  I did leave out a
> few (like the libc one) where the current page had multiple different versions.
>
> --
> Daniel Gustafsson
>
>
>