RE: Reword messages using "as" instead of "because"
От | Zhijie Hou (Fujitsu) |
---|---|
Тема | RE: Reword messages using "as" instead of "because" |
Дата | |
Msg-id | TY4PR01MB1690739EBFA0443A7A83028449417A@TY4PR01MB16907.jpnprd01.prod.outlook.com обсуждение исходный текст |
Ответ на | Re: Reword messages using "as" instead of "because" (Amit Kapila <amit.kapila16@gmail.com>) |
Ответы |
Re: Reword messages using "as" instead of "because"
|
Список | pgsql-hackers |
On Tuesday, September 16, 2025 11:56 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Sep 16, 2025 at 8:17 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> > wrote: > > > > I noticed that the recent commit 0d48d393d46 introduced the following > > three messages: > > > > 4793> errdetail("Retention is stopped as the apply process is not > > 4793> advancing its xmin within the configured max_retention_duration > > 4793> of %u ms.", > > 4822> ? errdetail("Retention is re-enabled as the apply process is > > 4822> advancing its xmin within the configured max_retention_duration > > 4822> of %u ms.", > > 4824> : errdetail("Retention is re-enabled as max_retention_duration > > 4824> is set to unlimited.")); > > > > I think I saw other instances of this kind of as recently, and I > > thought we had agreed to avoid this usage and prefer because instead, > > but I lost track of where that discussion took place. > > > > Anyway, unlike some past uses, these ones are apparently confusing, > > and I'd like to propose changing the wording to because. > > > > Thanks for pointing this out. I checked the code and found the use of 'because' > is preferred. +1 > > > In addition, I felt that the tense in the second message is not > > immediately clear. If it is reasonable and keeps the correct sense, > > I'd like to propose changing "is (not) advancing its xmin within" to > > "has (not) advanced its xmin into". > > > > + errdetail("Retention is stopped because the apply process has not > > + advanced its xmin into the configured max_retention_duration of %u > > + ms.", ? errdetail("Retention is re-enabled because the apply process > > + has advanced its xmin into the configured max_retention_duration of > > + %u ms.", > > + : errdetail("Retention is re-enabled because max_retention_duration > > + is set to unlimited.")); > > > > In the above sentence, has advanced sounds like we have already advanced > but that is not the case. Also, use of into looks odd to me. > How about changing it to: "Retention is re-enabled because the apply process > can advance its xmin within the configured max_retention_duration of %u > ms."? > > Similarly for the first message, how about: "Retention is stopped because the > apply process could not advance its xmin within the configured > max_retention_duration of %u ms."? I think the suggested message aligns better with the implementation. I updated the message based on Horiguchi-San's revision. Additionally, 035_conflicts.pl has been modified, as it was waiting for the resume DETAIL message and this message has now been updated. Best Regards, Hou zj
Вложения
В списке pgsql-hackers по дате отправления: