On Mon, Sep 23, 2024 at 7:57 AM Peter Smith <smithpb2...@gmail.com> wrote: > > My review comments for v8-0001 > > ====== > contrib/pg_logicalinspect/pg_logicalinspect.c > > 1. > +/* > + * Lookup table for SnapBuildState. > + */ > + > +#define SNAPBUILD_STATE_INCR 1 > + > +static const char *const SnapBuildStateDesc[] = { > + [SNAPBUILD_START + SNAPBUILD_STATE_INCR] = "start", > + [SNAPBUILD_BUILDING_SNAPSHOT + SNAPBUILD_STATE_INCR] = "building", > + [SNAPBUILD_FULL_SNAPSHOT + SNAPBUILD_STATE_INCR] = "full", > + [SNAPBUILD_CONSISTENT + SNAPBUILD_STATE_INCR] = "consistent", > +}; > + > +/* > > nit - the SNAPBUILD_STATE_INCR made this code appear more complicated > than it is.
I agree. > Please take a look at the attachment for an alternative > implementation which includes an explanatory comment. YMMV. Feel free > to ignore it. > +1. I find Peter's version with comments easier to understand. thanks Shveta