| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: A few message wording/formatting cleanup patches |
| Date: | 2026-05-28 17:19:19 |
| Message-ID: | 5276EEF9-7953-4EC4-8403-3DE0157100F4@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> Hello,
>
> While translating error messages into Japanese
Thank you so much for doing this!
Some comments on a few of your patches:
> 0002: 0002-Use-double-quotes-in-message.patch
>
> fe-protocol3.c has the following message containing backquotes:
>
>> libpq_append_conn_error(conn,
>> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
>
> but that doesn't seem to match our usual style. Nearby messages use
> double quotes instead.
Agreed. Shouldn't it also be adding the parameter as a %s to further match our style?
- libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
+ libpq_append_conn_error(conn, "server did not report the unsupported \"%s\" parameter in its protocol negotiation message", "_pq_.test_protocol_negotiation");
> 0003: 0003-Add-missing-period-to-HINT-message.patch
>
> In be-secure-openssl.c, the following HINT message is missing a
> trailing period:
>
>> errhint("If ssl_sni is enabled then add configuration to "%s", else "%s"",
My bad, will fix. Reading the hint it's also not as helpful as it could be for
a hint I think, perhaps this would be better?
- errhint("If ssl_sni is enabled then add configuration to \"%s\", else \"%s\"",
+ errhint("If ssl_sni is enabled then add configuration to \"%s\", else configure SSL in \"%s\".",
> 0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
>
> In datachecksum_state.c, the launcher process is referred to in two
> different ways: "datachecksum launcher" and "datachecksums
> launcher". Considering the worker process name, I think the former is
> probably the intended one, so this patch makes the naming consistent
> accordingly.
>
> That said, I can also imagine an interpretation where "datachecksums"
> was chosen intentionally to refer to the checksum feature or checksum
> set as a whole, so I'm not entirely sure whether this should be
> considered a real issue or just a stylistic inconsistency.
>
> Still, having both forms coexist seems somewhat error-prone in
> practice, especially when typing or searching symbol names.
I am clearly biased, or Stockholm syndromed, but DataChecksumsXXX was chosen
deliberately since it affects the feature which is exposed with the GUC
data_checksums. Renaming it does not improve clarity IMHO. The singular form
"checksum" is used where it refers to a single entity, like the cluster state
which can only be a single value.
It would however be an improvement to rename the "datachecksum launcher|worker"
cases you found to "datachecksums" since they are user facing.
- * This creates the list of databases for the datachecksumsworker workers to
+ * This creates the list of databases for the datachecksum workers to
This comment refers to a worker process of the type datachecksumsworker, the
proposed change makes that less clear IMO. That said, the original wording
isn't great so maybe "datachecksumsworker process" is better?
--
Daniel Gustafsson
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2026-05-28 17:23:24 | Re: A few message wording/formatting cleanup patches |
| Previous Message | Tristan Partin | 2026-05-28 17:11:42 | Re: Add pg_stat_kind_info system view |