Thread: Remove trailing comma from enums

I saw one (and then went looking and found some more) enum with a
trailing comma.

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.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
> I saw one (and then went looking and found some more) enum with a
> trailing comma.
>
> 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.

Thomas Munro <thomas.munro@gmail.com> writes:
> On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.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.

            regards, tom lane

On Thu, Jan 6, 2022 at 12:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Thomas Munro <thomas.munro@gmail.com> writes:
> > On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.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.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

At Thu, 6 Jan 2022 12:52:50 +1100, Peter Smith <smithpb2250@gmail.com> wrote in 
> On Thu, Jan 6, 2022 at 12:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Thomas Munro <thomas.munro@gmail.com> writes:
> > > On Thu, Jan 6, 2022 at 12:56 PM Peter Smith <smithpb2250@gmail.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