Hi,
On 2022-04-07 10:36:30 +0900, Kyotaro Horiguchi wrote:
> At Wed, 6 Apr 2022 09:04:09 -0700, Andres Freund <andres@anarazel.de> wrote in
> > > + * Don't define an INVALID value so switch() statements can warn if some
> > > + * cases aren't covered. But define the first member to 1 so that
> > > + * uninitialized values can be detected more easily.
> > >
> > > FWIW, I like this.
> >
> > I think there's no switches left now, so it's not actually providing too much.
>
> (Ouch!)
I think it's great that there's no switches left - means we're pretty close to
pgstat being runtime extensible...
> > > 0010:
> > > (I didn't look this closer. The comments arised while looking other
> > > patches.)
> > >
> > > +pgstat_kind_from_str(char *kind_str)
> > >
> > > I don't think I like "str" so much. Don't we spell it as
> > > "pgstat_kind_from_name"?
> >
> > name makes me think of NameData. What do you dislike about str? We seem to use
> > str in plenty places?
>
> For clarity, I don't dislike it so much. So, I'm fine with the
> current name.
>
> I found that you meant a type by the "str". I thought it as an
> instance (I'm not sure I can express my feeling correctly here..) and
> the following functions were in my mind.
>
> char *get_namespace/rel/collation/func_name(Oid someoid)
> char *pgstat_slru_name(int slru_idx)
>
> Another instance of the same direction is
>
> ForkNumber forkname_to_number(const char *forkName)
It's now pgstat_get_kind_from_str().
It was harder to see earlier (I certainly didn't really see it) - because
there were so many "violations" - but most of pgstat is
pgstat_<verb>_<subject>() or just <verb>_<subject>. I'd already moved most of
the patch series over to that (maybe in v68 or so). Now I also did that with
the internal functions.
There's a few functions breaking that pattern, partially because I added them
:(, but since they're not touched in these patches I've not renamed them. But
it's probably worth doing so tomorrow.
Greetings,
Andres Freund
