Re: Improve the connection failure error messages

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