typos

On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

     # Use larger ccache cache, as this task compiles with multiple compilers /
     # flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters. Perhaps somebody more
fluent with Cirrus, though, has a different opinion..

  * pointer to this structure.  The information here must be sufficient to
  * properly initialize each new TableScanDesc as workers join the scan, and it
- * must act as a information what to scan for those workers.
+ * must provide information what to scan for those workers.

This comment in 0009 is obviously incorrect, but I am not sure whether
your new suggestion is an improvement. Do workers provide such
information or has this structure some information that the workers
rely on?

Not sure that the whitespace issue in 0021 for the header of inval.c
is worth caring about.

0014 and 0013 do not reduce the translation workload, as the messages
include some stuff specific to the GUC names accessed to, or some
specific details about the code paths triggered.

The rest has been applied where they matter.
--
Michael

On Tue, Jan 3, 2023 at 12:58 PM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters.

The other places in that file use M, so maybe, this is more consistent.

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml?

--
With Regards,
Amit Kapila.

On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote:

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml?

Yeah, I was wondering as well why this change is not worse, which is
why I left it out of 33ab0a2. There is an acronym for UTF in
acronym.sgml, which makes sense to me, but that's the only place where
this is used. To add more on top of that, the docs basically need
only UTF8, and we have three references to UTF-16, none of them using
the <acronym> markup.
--
Michael

On 03.01.23 09:41, Michael Paquier wrote:

On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote:

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym> character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8 character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in func.sgml?

Yeah, I was wondering as well why this change is not worse, which is
why I left it out of 33ab0a2. There is an acronym for UTF in
acronym.sgml, which makes sense to me, but that's the only place where
this is used. To add more on top of that, the docs basically need
only UTF8, and we have three references to UTF-16, none of them using
the <acronym> markup.

The thing is called "UTF-8". Here, we are not talking about the
PostgreSQL identifier.

On Tue, Jan 03, 2023 at 04:28:29PM +0900, Michael Paquier wrote:

On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters. Perhaps somebody more
fluent with Cirrus, though, has a different opinion..

It's got almost nothing to do with cirrus. It's an environment
variable, and we're using a suffix other than what's
supported/documented by ccache, which only happens to work.

0014 and 0013 do not reduce the translation workload, as the messages
include some stuff specific to the GUC names accessed to, or some
specific details about the code paths triggered.

It seems to matter because otherwise the translators sometimes re-type
the view name, which (not surprisingly) can get messed up, which is how
I mentioned having noticed this.

On Tue, Jan 03, 2023 at 05:41:58PM +0900, Michael Paquier wrote:

On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote:

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym>
character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8
character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in
func.sgml?

Yeah, I was wondering as well why this change is not worse, which is
why I left it out of 33ab0a2. There is an acronym for UTF in
acronym.sgml, which makes sense to me, but that's the only place where
this is used. To add more on top of that, the docs basically need
only UTF8, and we have three references to UTF-16, none of them using
the <acronym> markup.

I changed it for consistency, as it's the only thing that says <>UTF-8<>
anywhere, and charset.sgml already says <>UTF<>-8 elsewhere.

Alternately, I suggest to change charset to say <>UTF8<> in both places.

--
Justin

On Tue, Jan 03, 2023 at 03:39:22PM -0600, Justin Pryzby wrote:

On Tue, Jan 03, 2023 at 04:28:29PM +0900, Michael Paquier wrote:

On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters. Perhaps somebody more
fluent with Cirrus, though, has a different opinion..

It's got almost nothing to do with cirrus. It's an environment
variable, and we're using a suffix other than what's
supported/documented by ccache, which only happens to work.

0014 and 0013 do not reduce the translation workload, as the messages
include some stuff specific to the GUC names accessed to, or some
specific details about the code paths triggered.

It seems to matter because otherwise the translators sometimes re-type
the view name, which (not surprisingly) can get messed up, which is how
I mentioned having noticed this.

On Tue, Jan 03, 2023 at 05:41:58PM +0900, Michael Paquier wrote:

On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote:

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym>
character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8
character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in
func.sgml?

Yeah, I was wondering as well why this change is not worse, which is
why I left it out of 33ab0a2. There is an acronym for UTF in
acronym.sgml, which makes sense to me, but that's the only place where
this is used. To add more on top of that, the docs basically need
only UTF8, and we have three references to UTF-16, none of them using
the <acronym> markup.

I changed it for consistency, as it's the only thing that says <>UTF-8<>
anywhere, and charset.sgml already says <>UTF<>-8 elsewhere.

Alternately, I suggest to change charset to say <>UTF8<> in both places.

As attached.
This also fixes "specualtive" in Amit's recent commit.

--
Justin

On Tue, Jan 10, 2023 at 10:27 AM Justin Pryzby <pryzby@telsasoft.com> wrote:

On Tue, Jan 03, 2023 at 03:39:22PM -0600, Justin Pryzby wrote:

On Tue, Jan 03, 2023 at 04:28:29PM +0900, Michael Paquier wrote:

On Fri, Dec 30, 2022 at 05:12:57PM -0600, Justin Pryzby wrote:

# Use larger ccache cache, as this task compiles with multiple compilers /
# flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1G"

In 0006, I am not sure how much this matters. Perhaps somebody more
fluent with Cirrus, though, has a different opinion..

It's got almost nothing to do with cirrus. It's an environment
variable, and we're using a suffix other than what's
supported/documented by ccache, which only happens to work.

0014 and 0013 do not reduce the translation workload, as the messages
include some stuff specific to the GUC names accessed to, or some
specific details about the code paths triggered.

It seems to matter because otherwise the translators sometimes re-type
the view name, which (not surprisingly) can get messed up, which is how
I mentioned having noticed this.

On Tue, Jan 03, 2023 at 05:41:58PM +0900, Michael Paquier wrote:

On Tue, Jan 03, 2023 at 01:03:01PM +0530, Amit Kapila wrote:

One minor comment:
-    spoken in Belgium (BE), with a <acronym>UTF-8</acronym>
character set
+    spoken in Belgium (BE), with a <acronym>UTF</acronym>-8
character set

Shouldn't this be <acronym>UTF8</acronym> as we are using in
func.sgml?

Yeah, I was wondering as well why this change is not worse, which is
why I left it out of 33ab0a2. There is an acronym for UTF in
acronym.sgml, which makes sense to me, but that's the only place where
this is used. To add more on top of that, the docs basically need
only UTF8, and we have three references to UTF-16, none of them using
the <acronym> markup.

I changed it for consistency, as it's the only thing that says <>UTF-8<>
anywhere, and charset.sgml already says <>UTF<>-8 elsewhere.

Alternately, I suggest to change charset to say <>UTF8<> in both places.

As attached.
This also fixes "specualtive" in Amit's recent commit.

Thanks for noticing this. I'll take care of this and some other typo
patches together.

--
With Regards,
Amit Kapila.

On Tue, Jan 10, 2023 at 12:24:40PM +0530, Amit Kapila wrote:

Thanks for noticing this. I'll take care of this and some other typo
patches together.

Does this include 0010? I was just looking at the whole set and this
one looked like a cleanup worth on its own so I was going to handle
it, until I saw your update. If you are also looking at that, I won't
stand in your way, of course :)
--
Michael

On Tue, Jan 10, 2023 at 1:18 PM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Jan 10, 2023 at 12:24:40PM +0530, Amit Kapila wrote:

Thanks for noticing this. I'll take care of this and some other typo
patches together.

Does this include 0010? I was just looking at the whole set and this
one looked like a cleanup worth on its own so I was going to handle
it, until I saw your update. If you are also looking at that, I won't
stand in your way, of course :)

I have not yet started, so please go ahead.

--
With Regards,
Amit Kapila.

On Tue, Jan 10, 2023 at 01:55:56PM +0530, Amit Kapila wrote:

I have not yet started, so please go ahead.

Okay, I have looked at that and fixed the whole new things, including
the typo you have introduced. 0001~0004 have been left out, as of the
same reasons as upthread.
--
Michael

Some more accumulated/new typos.

On Wed, Feb 08, 2023 at 09:56:44AM -0600, Justin Pryzby wrote:

Some more accumulated/new typos.

0001 has been a debate for a long time, and it depends on the way SQL
is spelled. For reference:
$ git grep -i " an sql" -- *.c | wc -l
63
$ git grep -i " a sql" -- *.c | wc -l
135

0005 can indeed fix a lot of confusion around the spaces after an
"else if" block. Is that something that could be automated with the
indentation, though? Same remark for 0009 and 0010.

Applied 0002, 0003, 0004, 0006, after rewording a bit 0003 to mention
the compression type.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Wed, Feb 08, 2023 at 09:56:44AM -0600, Justin Pryzby wrote:

Some more accumulated/new typos.

0005 can indeed fix a lot of confusion around the spaces after an
"else if" block. Is that something that could be automated with the
indentation, though? Same remark for 0009 and 0010.

I see your point about 0005, but I've never seen pgindent remove
vertical whitespace once it's been added. Not sure what it'd take
to teach it to do so, or whether we'd like the results.

I'd reject 0009 and 0010 altogether --- they don't add any readability
that's worth the potential increase in back-patch problems.

regards, tom lane