Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Wed, Sep 9, 2015 at 11:49 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> - errdetail("Could not rename \"%s\" to \"%s\": %m.",
>> + errdetail("\"%s\" could not be renamed to \"%s\": %m.",
>>
>> Is there any reason to change this message?
>> I think you have changed this message to make it somewhat similar with
>> the new message we are planning to use in this function, but I don't see
>> that as compelling reason to change this message.

> The old message better follows the guidelines. See section 51.3.7:
> Avoid Passive Voice. The old message is what's called
> "telegram-style", with PostgreSQL itself as the implicit subject. The
> proposed replacement is just the regular old passive voice.

Neither version is following the guidelines very well, in particular they
should be mentioning what kind of object %s is (file? table? tablespace?).
But to me the "could not be renamed" version seems to be closer to the
spirit of the "use complete sentences" rule for errdetail. The other one
seems better fit for a primary error message, which is supposed to be
kept short.

regards, tom lane