Re: Corrected documentation of data type for the logical replication message formats.

On Wed, May 12, 2021 at 3:36 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Wed, May 12, 2021 at 1:02 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > Thanks for the comments, Attached v3 patch has the changes as suggested.
>
> This v3 mostly looks good to me now except for some minor comments
> about the flags.
>
> ~~~
>
> 1. Commit flags
>
> @@ -6534,11 +6536,11 @@ Commit
>  </varlistentry>
>  <varlistentry>
>  <term>
> -        Int8
> +        Uint8(0)
>  </term>
>  <listitem>
>  <para>
> -                Flags; currently unused (must be 0).
> +                Flags (uint8(0)); currently unused (must be 0).
>  </para>
>  </listitem>
>  </varlistentry>
>
> a) There is no data type Uint8. That should be "Int8(0)"
>

Modified.

> b) I think the "(0)" does not belong in the description part.
> e.g. change to "Flags (uint8); currently unused (must be 0)."
>

Modified

> ~~~
>
> 2. Stream Commit flags
>
> @@ -7276,7 +7284,7 @@ Stream Commit
>  </term>
>  <listitem>
>  <para>
> -                Flags; currently unused (must be 0).
> +                Flags (uint8(0)); currently unused (must be 0).
>  </para>
>  </listitem>
>  </varlistentry>
>
> a) The data type should say "Int8(0)"
>

Modified.

> b) I think the "(0)" does not belong in the description part.
> e.g. change to "Flags (uint8); currently unused (must be 0)."
>

Modified.

> ~~~
>
> 3. I felt that saying "(must be 0)" for those unused flag descriptions
> is unnecessary since that is exactly what "Int8(0)" already means.
> e.g. consider change to just say "Flags (uint8); currently unused."

Modified.

Thanks for the comments. Attached v4 patch has the fix for the same.

Regards,
Vignesh