On 2020-06-12 09:30, Michael Paquier wrote:
On Thu, Jun 11, 2020 at 03:55:59PM +0200, Peter Eisentraut wrote:
On 2020-01-16 13:56, Robert Haas wrote:
IMHO, custom enums for each particular case would be a big improvement
over supposedly-generic STATUS codes. It makes it clearer which values
are possible in each code path, and it comes out nicer in the
debugger, too.

Given this feedback, I would like to re-propose the original patch, attached
again here.

After this, the use of the remaining STATUS_* symbols will be contained to
the frontend and backend libpq code, so it'll be more coherent.

I am still in a so-so state regarding this patch, but I find the
debugger argument a good one.  And please don't consider me as a
blocker.

Okay, I have committed it.

Add a separate enum for use in the locking APIs, which were the only
user.

+typedef enum
+{
+       PROC_WAIT_STATUS_OK,
+       PROC_WAIT_STATUS_WAITING,
+       PROC_WAIT_STATUS_ERROR,
+} ProcWaitStatus;

ProcWaitStatus, and more particularly PROC_WAIT_STATUS_WAITING are
strange names (the latter refers to "wait" twice).  What do you think
about renaming the enum to ProcStatus and the flags to PROC_STATUS_*?

I see your point, but I don't think that's better. That would just invite someone else to use it for other process-related status things. We typically name enum constants like the type followed by a suffix. The fact that the suffix is similar to the prefix here is more of a coincidence.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply via email to