Content-Type: text/plain;
	charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.3\))
Subject: Re: Improve the log message output of basic_archive when
 basic_archive.archive_directory parameter is not set
From: Daniel Gustafsson <daniel@yesql.se>
In-Reply-To: <85b9e69997bbd5d8302a3a99a2de246d@oss.nttdata.com>
Date: Wed, 20 Sep 2023 14:14:54 +0200
Cc: Nathan Bossart <nathandbossart@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 pgsql-hackers@lists.postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <D3038255-EAB5-4164-A9E3-7967B6738993@yesql.se>
References: <202309151049.dmfsxc5b3wyf@alvherre.pgsql>
 <7035FAFE-7E91-4B31-AA72-75EE82DB3F7B@yesql.se>
 <20230915143827.GA1912920@nathanxps13>
 <85b9e69997bbd5d8302a3a99a2de246d@oss.nttdata.com>
To: bt23nguyent <bt23nguyent@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/D3038255-EAB5-4164-A9E3-7967B6738993%40yesql.se>
Precedence: bulk

> On 19 Sep 2023, at 11:21, bt23nguyent <bt23nguyent@oss.nttdata.com> =
wrote:

> Please let me know if you have any further suggestions that I can =
improve more.

+        *logdetail =3D 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].

-    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.=20

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=