On Tue, Jul 9, 2024 at 1:00 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> Nisha Moond <nisha.moond...@gmail.com> writes:
> > Attached v5 patch with the translator comments as suggested.
>
> I looked at this, and I agree with the goal, but I find just about all
> of the translator comments unnecessary. The ones that are useful are
> useful only because the message is violating one of our message style
> guidelines [1]:
>
> When citing the name of an object, state what kind of object it is.
>
> Rationale: Otherwise no one will know what “foo.bar.baz” refers to.
>
> So, for example, where you have
>
> +
> + /*
> + * translator: first %s is the subscription name, second %s is
> the
> + * error
> + */
> + errmsg("subscription \"%s\" could not connect to the
> publisher: %s", stmt->subname, err)));
>
> I don't find that that translator comment is adding anything.
> But there are a couple of places like
>
> + /*
> + * translator: first %s is the slotsync worker name, second %s is the
> + * error
> + */
> + errmsg("\"%s\" could not connect to the primary server: %s",
> app_name.data, err));
>
> I think that the right cure for the ambiguity here is not to add a
> translator comment, but to label the name properly, perhaps like
>
> errmsg("synchronization worker \"%s\" could not connect to
> the primary server: %s",
> app_name.data, err));
>
>
> regards, tom lane
>
> [1]
> https://www.postgresql.org/docs/devel/error-style-guide.html#ERROR-STYLE-GUIDE-OBJECT-TYPEThank you for the review. Attached the patch v6 with suggested improvements. - Removed unnecessary translator comments. - Added appropriate identifier names where missing. -- Thanks, Nisha
v6-0001-Improve-the-connection-failure-error-messages.patch
Description: Binary data
