[FFmpeg-devel] comma at the end of enumerator lists

Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
>> On Mon, 17 Oct 2016 13:09:36 +0200
>> Michael Niedermayer <michael@niedermayer.cc> wrote:
>>
>>> On Mon, Oct 17, 2016 at 10:07:42AM +0200, Nicolas George wrote:
>>>> Le sextidi 26 vendémiaire, an CCXXV, Michael Niedermayer a écrit :  
>>>>> probably, yes  
>>>>
>>>> I would have said exactly the opposite. It is nothing but a waste of time
>>>> and a pollution of the history.  
>>>
>>> My idea here is to maximize the number of developers
>>> And if in cases where one doesnt really care much either way and
>>> someone else seems caring more one says, "ok" that may result in a happy
>>> new contributor.
>>> Saying "no" is more likely to turn someone away.
>>> and again, it doesnt really matter if the , is there after a
>>> final sentinel /count entry as no next field would ever be added
>>
>> Are you kidding me. Patches should be judged on their technical merrit,
>> not whether you might piss someone off by rejecting it.
> 
> this is about a cosmetic change having no real technical effect

So here are my cosmetics for libavutil. It simply helps with keeping
track of real warnings in downstream projects.

Best regards,
Michael

Hi Michael,

On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch <oss@behrisch.de> wrote:

> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
> > On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
> >> On Mon, 17 Oct 2016 13:09:36 +0200
> >> Michael Niedermayer <michael@niedermayer.cc> wrote:
> >>
> >>> On Mon, Oct 17, 2016 at 10:07:42AM +0200, Nicolas George wrote:
> >>>> Le sextidi 26 vendémiaire, an CCXXV, Michael Niedermayer a écrit :
> >>>>> probably, yes
> >>>>
> >>>> I would have said exactly the opposite. It is nothing but a waste of
> time
> >>>> and a pollution of the history.
> >>>
> >>> My idea here is to maximize the number of developers
> >>> And if in cases where one doesnt really care much either way and
> >>> someone else seems caring more one says, "ok" that may result in a
> happy
> >>> new contributor.
> >>> Saying "no" is more likely to turn someone away.
> >>> and again, it doesnt really matter if the , is there after a
> >>> final sentinel /count entry as no next field would ever be added
> >>
> >> Are you kidding me. Patches should be judged on their technical merrit,
> >> not whether you might piss someone off by rejecting it.
> >
> > this is about a cosmetic change having no real technical effect
>
> So here are my cosmetics for libavutil. It simply helps with keeping
> track of real warnings in downstream projects.


Why are you using -Wpedantic?

Most people use warnings as a way for the compiler to inform them of
potential bugs in their code; has -Wpedantic ever helped you find bugs?

Ronald

Hi Ronald,

Am 17.10.2016 um 21:37 schrieb Ronald S. Bultje:
> Hi Michael,
> 
> On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch <oss@behrisch.de>
> wrote:
> 
>> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
>>> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
>>>> On Mon, 17 Oct 2016 13:09:36 +0200 Michael Niedermayer
>>>> <michael@niedermayer.cc> wrote:
>>>> 
>>> this is about a cosmetic change having no real technical effect
>> 
>> So here are my cosmetics for libavutil. It simply helps with
>> keeping track of real warnings in downstream projects.
> 
> 
> Why are you using -Wpedantic?

My main reason is that we are compiling with different compilers for
different platforms and -Wpedantic at least promises to keep the code
closer to the standard and thus better transferable. I never tested
whether this is actually true, but I like the fact that the project
currently compiles with gcc, clang and msvc and welcome every tool and
option that helps me to keep it this way. See also here:
http://stackoverflow.com/questions/2855121/what-is-the-purpose-of-using-pedantic-in-gcc-g-compiler

> 
> Most people use warnings as a way for the compiler to inform them of 
> potential bugs in their code; has -Wpedantic ever helped you find
> bugs?

I cannot think of any but to be honest I cannot even tell exactly which
warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags
and it is surprisingly hard to find out.

Best regards,
Michael

Hi Michael,

On Mon, Oct 17, 2016 at 5:23 PM, Michael Behrisch <oss@behrisch.de> wrote:

> Hi Ronald,
>
> Am 17.10.2016 um 21:37 schrieb Ronald S. Bultje:
> > Hi Michael,
> >
> > On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch <oss@behrisch.de>
> > wrote:
> >
> >> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
> >>> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
> >>>> On Mon, 17 Oct 2016 13:09:36 +0200 Michael Niedermayer
> >>>> <michael@niedermayer.cc> wrote:
> >>>>
> >>> this is about a cosmetic change having no real technical effect
> >>
> >> So here are my cosmetics for libavutil. It simply helps with
> >> keeping track of real warnings in downstream projects.
> >
> >
> > Why are you using -Wpedantic?
>
> My main reason is that we are compiling with different compilers for
> different platforms and -Wpedantic at least promises to keep the code
> closer to the standard and thus better transferable. I never tested
> whether this is actually true, but I like the fact that the project
> currently compiles with gcc, clang and msvc and welcome every tool and
> option that helps me to keep it this way. See also here:
> http://stackoverflow.com/questions/2855121/what-is-the-
> purpose-of-using-pedantic-in-gcc-g-compiler
>
> >
> > Most people use warnings as a way for the compiler to inform them of
> > potential bugs in their code; has -Wpedantic ever helped you find
> > bugs?
>
> I cannot think of any but to be honest I cannot even tell exactly which
> warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags
> and it is surprisingly hard to find out.


FFmpeg compiles with msvc, clang and gcc (without -Wpedantic), so the logic
seems a little strange.

Ronald

Hi Ronald,

Am 17.10.2016 um 23:45 schrieb Ronald S. Bultje:
> Hi Michael,
> 
> On Mon, Oct 17, 2016 at 5:23 PM, Michael Behrisch <oss@behrisch.de> wrote:
> 
>> Hi Ronald,
>>
>> Am 17.10.2016 um 21:37 schrieb Ronald S. Bultje:
>>> Hi Michael,
>>>
>>> On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch <oss@behrisch.de>
>>> wrote:
>>>
>>>> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
>>>>> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
>>>>>> On Mon, 17 Oct 2016 13:09:36 +0200 Michael Niedermayer
>>>>>> <michael@niedermayer.cc> wrote:
>>>>>>
>>>>> this is about a cosmetic change having no real technical effect
>>>>
>>>> So here are my cosmetics for libavutil. It simply helps with
>>>> keeping track of real warnings in downstream projects.
>>>
>>>
>>> Why are you using -Wpedantic?
>>
>> My main reason is that we are compiling with different compilers for
>> different platforms and -Wpedantic at least promises to keep the code
>> closer to the standard and thus better transferable. I never tested
>> whether this is actually true, but I like the fact that the project
>> currently compiles with gcc, clang and msvc and welcome every tool and
>> option that helps me to keep it this way. See also here:
>> http://stackoverflow.com/questions/2855121/what-is-the-
>> purpose-of-using-pedantic-in-gcc-g-compiler
>>
>>>
>>> Most people use warnings as a way for the compiler to inform them of
>>> potential bugs in their code; has -Wpedantic ever helped you find
>>> bugs?
>>
>> I cannot think of any but to be honest I cannot even tell exactly which
>> warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags
>> and it is surprisingly hard to find out.
> 
> 
> FFmpeg compiles with msvc, clang and gcc (without -Wpedantic), so the logic
> seems a little strange.

I am not claiming that -Wpedantic is the only way to achieve this. I
only think it helps or at least I assume that was the intention when it
was invented. After all, that is one of the reasons why standards exist
(in my opinion), to keep code compiler agnostic such that (in theory) I
do not have to check by myself whether it compiles with all the
compilers out there, as long as it adheres to the standard. And
"-Wpedantic" is just a "standard" checker to me. Maybe it checks for an
outdated version of the standard or for some version which is of no
relevance to FFmpeg, then please tell me.

Best regards,
Michael

2016-10-18 0:17 GMT+02:00 Michael Behrisch <oss@behrisch.de>:

> I am not claiming that -Wpedantic is the only way to achieve this.
> I only think it helps or at least I assume that was the intention
> when it was invented.

("If gcc does it this way, it must be a good idea.")

This is an extremely unconvincing argument and concerning your
next sentence, I would expect that the trailing commas do not
violate any standard (if they do, your original patch would most
likely be ok).

In any case, your second patch is acceptable, should not have
any adversary effects and we have significantly worse cosmetic
patches in our tree, so it is ok imo.

Carl Eugen

On Tue, Oct 18, 2016 at 00:21:42 +0200, Carl Eugen Hoyos wrote:
> In any case, your second patch is acceptable, should not have
> any adversary effects and we have significantly worse cosmetic
> patches in our tree, so it is ok imo.

I only see one patch - did I miss one? - , and if that's the one you're
referring to, it should at least be created with git format-patch (and
thus include a proper commit message).

Moritz

On Mon, Oct 17, 2016 at 23:23:31 +0200, Michael Behrisch wrote:
> My main reason is that we are compiling with different compilers for
> different platforms

If you're trying to keep ffmpeg more portable, you could at least try
the real-life thing and actually attempt to build with Sun Studio or
newer ICC or whatever you have in mind, and report back what actual
problems they pose. That said, have you seen the amount of
platforms+compilers already covered in fate alone?

> I cannot think of any but to be honest I cannot even tell exactly which
> warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags
> and it is surprisingly hard to find out.

I agree. I also agree warnings should be eliminated, but some are just
useless or wrong or whatever, so often nothing to worry about if you
explicitly switched them on.

Moritz

2016-10-18 13:11 GMT+02:00 Moritz Barsnick <barsnick@gmx.net>:
> On Tue, Oct 18, 2016 at 00:21:42 +0200, Carl Eugen Hoyos wrote:
>> In any case, your second patch is acceptable, should not have
>> any adversary effects and we have significantly worse cosmetic
>> patches in our tree, so it is ok imo.
>
> I only see one patch - did I miss one? - , and if that's the one you're

I meant this one on the mailing list contrary to the first - inacceptable
- one on github, sorry.

> referring to, it should at least be created with git format-patch (and
> thus include a proper commit message).

Of course.

Carl Eugen