Horton

Thread

Commits

  1. Add "Add trailing commas to enum definitions" to .git-blame-ignore-revs

  2. Add trailing commas to enum definitions

  1. Add trailing commas to enum definitions

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-23T06:30:28Z

    Since C99, there can be a trailing comma after the last value in an enum 
definition.  A lot of new code has been introducing this style on the 
fly.  I have noticed that some new patches are now taking an 
inconsistent approach to this.  Some add the last comma on the fly if 
they add a new last value, some are trying to preserve the existing 
style in each place, some are even dropping the last comma if there was 
one.  I figured we could nudge this all in a consistent direction if we 
just add the trailing commas everywhere once.  See attached patch; it 
wasn't actually that much.

I omitted a few places where there was a fixed "last" value that will 
always stay last.  I also skipped the header files of libpq and ecpg, in 
case people want to use those with older compilers.  There were also a 
small number of cases where the enum type wasn't used anywhere (but the 
enum values were), which ended up confusing pgindent a bit.

  2. Re: Add trailing commas to enum definitions

    Junwang Zhao <zhjwpku@gmail.com> — 2023-10-23T09:55:32Z

    On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> Since C99, there can be a trailing comma after the last value in an enum

C99 allows us to do this doesn't mean we must do this, this is not
inconsistent IMHO, and this will pollute the git log messages, people
may *git blame* the file and see the reason for the introduction of the
line.

There are a lot of 'typedef struct' as well as 'struct', which is not
inconsistent either just like the *enum* case.

> definition.  A lot of new code has been introducing this style on the
> fly.  I have noticed that some new patches are now taking an
> inconsistent approach to this.  Some add the last comma on the fly if
> they add a new last value, some are trying to preserve the existing
> style in each place, some are even dropping the last comma if there was
> one.  I figured we could nudge this all in a consistent direction if we
> just add the trailing commas everywhere once.  See attached patch; it
> wasn't actually that much.
>
> I omitted a few places where there was a fixed "last" value that will
> always stay last.  I also skipped the header files of libpq and ecpg, in
> case people want to use those with older compilers.  There were also a
> small number of cases where the enum type wasn't used anywhere (but the
> enum values were), which ended up confusing pgindent a bit.



-- 
Regards
Junwang Zhao

  3. Re: Add trailing commas to enum definitions

    Nathan Bossart <nathandbossart@gmail.com> — 2023-10-23T20:34:32Z

    On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
> On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> Since C99, there can be a trailing comma after the last value in an enum
> 
> C99 allows us to do this doesn't mean we must do this, this is not
> inconsistent IMHO, and this will pollute the git log messages, people
> may *git blame* the file and see the reason for the introduction of the
> line.

I suspect that your concerns about git-blame could be resolved by adding
this commit to .git-blame-ignore-revs.  From a long-term perspective, I
think standardizing on the trailing comma style will actually improve
git-blame because patches won't need to add a comma to the previous line
when adding a value.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  4. Re: Add trailing commas to enum definitions

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-23T21:04:08Z

    Nathan Bossart <nathandbossart@gmail.com> writes:
> From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

Yeah, that's a good point.  I had been leaning towards "this is
unnecessary churn", but with that idea I'm now +1.

			regards, tom lane

  5. Re: Add trailing commas to enum definitions

    David Steele <david@pgmasters.net> — 2023-10-23T23:58:23Z

    On 10/23/23 17:04, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>>  From a long-term perspective, I
>> think standardizing on the trailing comma style will actually improve
>> git-blame because patches won't need to add a comma to the previous line
>> when adding a value.
> 
> Yeah, that's a good point.  I had been leaning towards "this is
> unnecessary churn", but with that idea I'm now +1.

+1 from me.

-David

  6. Re: Add trailing commas to enum definitions

    Junwang Zhao <zhjwpku@gmail.com> — 2023-10-24T06:07:29Z

    On Tue, Oct 24, 2023 at 4:34 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
> > On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> >> Since C99, there can be a trailing comma after the last value in an enum
> >
> > C99 allows us to do this doesn't mean we must do this, this is not
> > inconsistent IMHO, and this will pollute the git log messages, people
> > may *git blame* the file and see the reason for the introduction of the
> > line.
>
> I suspect that your concerns about git-blame could be resolved by adding
> this commit to .git-blame-ignore-revs.  From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

make sense, +1 from me now.

>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com



-- 
Regards
Junwang Zhao

  7. Re: Add trailing commas to enum definitions

    Andrew Dunstan <andrew@dunslane.net> — 2023-10-24T12:58:38Z

    On 2023-10-23 Mo 17:04, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>>  From a long-term perspective, I
>> think standardizing on the trailing comma style will actually improve
>> git-blame because patches won't need to add a comma to the previous line
>> when adding a value.
> Yeah, that's a good point.  I had been leaning towards "this is
> unnecessary churn", but with that idea I'm now +1.
>
> 			


+1. It's a fairly common practice in Perl code, too, and I often do it 
for exactly this reason.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  8. Re: Add trailing commas to enum definitions

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-26T11:20:34Z

    On 23.10.23 22:34, Nathan Bossart wrote:
> On Mon, Oct 23, 2023 at 05:55:32PM +0800, Junwang Zhao wrote:
>> On Mon, Oct 23, 2023 at 2:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>>> Since C99, there can be a trailing comma after the last value in an enum
>>
>> C99 allows us to do this doesn't mean we must do this, this is not
>> inconsistent IMHO, and this will pollute the git log messages, people
>> may *git blame* the file and see the reason for the introduction of the
>> line.
> 
> I suspect that your concerns about git-blame could be resolved by adding
> this commit to .git-blame-ignore-revs.  From a long-term perspective, I
> think standardizing on the trailing comma style will actually improve
> git-blame because patches won't need to add a comma to the previous line
> when adding a value.

Committed that way.