Message-ID: <26fd4e28-8b75-4e61-a3fd-aed3b7442606@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Fix logging for invalid recovery timeline
To: =?UTF-8?Q?Benoit_Lobr=C3=A9au?= <benoit.lobreau@dalibo.com>,
 Michael Paquier <michael@paquier.xyz>
Cc: "Andrey M. Borodin" <x4mmm@yandex-team.ru>,
 Pg Hackers <pgsql-hackers@postgresql.org>
References: <c3d617d4-1696-4aa7-8a4d-5a7d19cc5618@pgbackrest.org>
 <C748635E-B2FF-4813-B035-AE670832B9FD@yandex-team.ru>
 <d0b27c08-d30f-4a1d-990d-5c90ca0a650a@pgbackrest.org>
 <4ed10bc8-5513-4d8e-8643-8abcaa08336d@dalibo.com>
 <2765f538-b06f-4e57-bcb2-9ca1dc6f487d@pgbackrest.org>
 <Z7aJU2_ZAplbpTsL@paquier.xyz>
 <6273ef0b-033b-4ac7-ab05-71a04e7ac626@pgbackrest.org>
 <173a9a4a-bad3-4a42-91e1-637f70e9512f@dalibo.com>
 <5336e68a-30d0-473f-a0a0-9e61cd9a5a63@pgbackrest.org>
 <Z7vF1hyg5JOFTWst@paquier.xyz>
 <f0e04cf5-e7e3-4c16-9c7a-dc7a2aa2c069@dalibo.com>
Content-Language: en-US
From: David Steele <david@pgbackrest.org>
In-Reply-To: <f0e04cf5-e7e3-4c16-9c7a-dc7a2aa2c069@dalibo.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Date: Mon, 24 Feb 2025 17:30:35 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/26fd4e28-8b75-4e61-a3fd-aed3b7442606%40pgbackrest.org>
Precedence: bulk

On 2/24/25 10:21, Benoit Lobréau wrote:
> On 2/24/25 2:05 AM, Michael Paquier wrote:
>> I think that you have the right idea here, avoiding the duplication
>> of the errdetail() which feels itchy when looking at the patch. 
> 
> Done this way in the attached patch.

This looks good to me.

>> This should have a note for translators that this field refers to a file
>> name. 
> 
> Is there a specific way todo this?

If you search the source for translator: you'll find lots of examples. 
Here's one that was pretty relevant:

/* translator: %s is a configuration file */

We could probably phrase that as:

/* translator: %s is a backup_label or pg_control file */

That note goes right above the hint, so something like:

+                                /* translator: %s is a backup_label or 
pg_control file */
+				 errdetail("Latest checkpoint in %s is at %X/%X on timeline %u, but 
in the history of the requested timeline, the server forked off from 
that timeline at %X/%X.",
+						   haveBackupLabel ? "backup_label" : "pg_control",

Regards,
-David