On Tue, Jul 9, 2024 at 1:00 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Nisha Moond <nisha.moond412@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-TYPE
Thank you for the review.
Attached the patch v6 with suggested improvements.
- Removed unnecessary translator comments.
- Added appropriate identifier names where missing.
--
Thanks,
Nisha