Inconsistent GUC descriptions
Hello, (this mail is not a duplicate sent by mistake.)
The recent commit ac0e33136ab introduced the following message:
+ GUC_check_errdetail("The value of \"%s\" must be set to 0 during binary upgrade mode.",
However, the existing message for the same situation is written
without "The value of" at the beginning. In addition, all existing
messages following the "%s must be set to" pattern omit this phrase.
Therefore, I believe the initial part of the new message should be
removed for consistency. The attached patch makes this adjustment.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
0001-Align-a-recently-added-message-with-an-existing-one.patchtext/x-patch; charset=us-asciiDownload
From 6e21379ebbcf91da69ea13e8e20aab7984a71ff7 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 20 Feb 2025 13:31:52 +0900
Subject: [PATCH] Align a recently added message with an existing one
A recently added message follows the style "The value of "%s" must be
set", but for this situation, the existing message uses "%s" must be
set" instead. In addition, all instances of "%s" must be set are
written without the preceding "The value of".
This patch removes the inconsistency by adjusting the message to match
the existing one.
---
src/backend/replication/slot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 292407f5149..84270c493a5 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2986,7 +2986,7 @@ check_idle_replication_slot_timeout(int *newval, void **extra, GucSource source)
{
if (IsBinaryUpgrade && *newval != 0)
{
- GUC_check_errdetail("The value of \"%s\" must be set to 0 during binary upgrade mode.",
+ GUC_check_errdetail("\"%s\" must be set to 0 during binary upgrade mode.",
"idle_replication_slot_timeout");
return false;
}
--
2.43.5
Dear Horiguchi-san,
I really appreciate your post-commit reviewing.
However, the existing message for the same situation is written
without "The value of" at the beginning.
Right. To clarify, max_slot_wal_keep_size has similar check hook which rejects
the upgrade mode, and it starts with "\"%s\" must be set to".
In addition, all existing
messages following the "%s must be set to" pattern omit this phrase.
Yes, effective_io_concurrency and maintenance_io_concurrency start with it.
Therefore, I believe the initial part of the new message should be
removed for consistency. The attached patch makes this adjustment.
Agreed. I read your patch just in case and looks good to me...
Best regards,
Hayato Kuroda
FUJITSU LIMITED