Re: Remove trailing comma from enums

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: smithpb2250(at)gmail(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, thomas(dot)munro(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Remove trailing comma from enums
Date: 2022-01-06 08:20:12
Message-ID: 20220106.172012.1399671609872960405.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Thu, 6 Jan 2022 12:52:50 +1100, Peter Smith <smithpb2250(at)gmail(dot)com> wrote in
> On Thu, Jan 6, 2022 at 12:23 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> > > On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >> These are quite rare in the PG src, so I doubt they are intentional.
> > >> PSA a patch to remove the trailing commas for all that I found.
> >
> > > -1. I don't see the problem with C99 trailing commas. They avoid
> > > noisy diff lines when patches add/remove items.
> >
> > I think they're rare because up till very recently we catered to
> > pre-C99 compilers that wouldn't accept them. There's not much
> > point in insisting on that now, though.
> >
> > Personally I'm less excited than Thomas about trailing commas
> > being good for reducing diff noise, mainly because I think
> > that "add new entries at the end" is an anti-pattern, and
> > if you put new items where they logically belong then the
> > problem is much rarer. But I'm not going to argue against
> > committers who want to do it like that, either.
>
> FWIW, the background of this was that one of these examples overlapped
> with a feature currently in development and it just caused a waste of
> everyone's time by firstly "fixing" (removing) the extra comma and
> then getting multiple code reviews saying the change was unrelated to
> that feature and so having to remove that fix again. So I felt
> removing all such commas at HEAD not only makes all the enums
> consistent, but it may prevent similar time-wasting for others in the
> future.

I don't know where the above conversation took place, but it seems to
me that the first patch is not significant for reviewing and the last
patch seems to be just a waste of time even premising the first patch
survives.

I don't care whether the last item of an enum has a trailing comma or
not. (Or I like comma-less generally but I understand Thomas'
opinion.) I think one may take either way if need to modify the lines
involving such lines. But mildly object to make a change just to fix
them.

# Also, I don't want to see a comma-batttle breaks out at the end of
# an enum though...

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2022-01-06 08:29:23 Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit
Previous Message Dilip Kumar 2022-01-06 08:13:27 Re: Make relfile tombstone files conditional on WAL level