Re: Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set

On 2023-09-20 21:14, Daniel Gustafsson wrote:
>> On 19 Sep 2023, at 11:21, bt23nguyent <bt23nguyent(at)oss(dot)nttdata(dot)com>
>> wrote:
>
>> Please let me know if you have any further suggestions that I can
>> improve more.
>
> + *logdetail = pstrdup("WAL archiving failed because
> basic_archive.archive_directory is not set");
>
> Nitpick: detail messages should end with a period per the error message
> style
> guide [0].
>

Yes! I totally missed this detail.

> - archiving will proceed only when it returns
> <literal>true</literal>.
> + archiving will proceed only when it returns
> <literal>true</literal>. The
> + archiver may also emit the detail explaining how the module is
> not configured
> + to the sever log if the archive module has any.
>
> I think this paragraph needs to be updated to include how the returned
> logdetail is emitted, since it currently shows the WARNING without
> mentioning
> the added detail in case returned. It would also be good to mention
> that it
> should be an allocated string which the caller can free.
>
> --
> Daniel Gustafsson
>
> [0] https://www.postgresql.org/docs/devel/error-style-guide.html

Thank you for your kind review comment!

I agree with you that this document update is not explanatory enough.
So here is an updated patch.

If there is any further suggestion, please let me know.

Best regards,
Tung Nguyen