[FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 doc/developer.texi | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index a1bfe180c9b..6a753f99da6 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
 @end example
 
 @item
-No braces around single-line blocks:
+No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
 
 @example c, good
 // Good
-if (bits_pixel == 24)
+if (bits_pixel == 24) @{
     avctx->pix_fmt = AV_PIX_FMT_BGR24;
-else if (bits_pixel == 8)
+@} else if (bits_pixel == 8) @{
     avctx->pix_fmt = AV_PIX_FMT_GRAY8;
-else @{
-    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
+@} else
     return AVERROR_INVALIDDATA;
-@}
+
 @end example
 
 @item
-- 
2.48.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[epirat07]

On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:

> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  doc/developer.texi | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a1bfe180c9b..6a753f99da6 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>  @end example
>
>  @item
> -No braces around single-line blocks:
> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>

I agree with this, however people in the past pointed it out when new code did this, IIRC.

So if we merge this, people should stop flagging this in reviews, else its frustrating
for new contributors following the documentation and then getting contradicting reviews.

>  @example c, good
>  // Good
> -if (bits_pixel == 24)
> +if (bits_pixel == 24) @{
>      avctx->pix_fmt = AV_PIX_FMT_BGR24;
> -else if (bits_pixel == 8)
> +@} else if (bits_pixel == 8) @{
>      avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> -else @{
> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> +@} else
>      return AVERROR_INVALIDDATA;
> -@}
> +
>  @end example
>
>  @item
> -- 
> 2.48.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Vittorio Giovara]

On Thu, Feb 27, 2025 at 11:47 PM <epirat07@gmail.com> wrote:

> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/developer.texi | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >  @end example
> >
> >  @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
> >
>
> I agree with this, however people in the past pointed it out when new code
> did this, IIRC.
>
> So if we merge this, people should stop flagging this in reviews, else its
> frustrating
> for new contributors following the documentation and then getting
> contradicting reviews.


would loooove a format defining script with a prehook that formats your
patches before sending <3
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1326 bytes --]

On 2/27/2025 7:53 PM, Vittorio Giovara wrote:
> On Thu, Feb 27, 2025 at 11:47 PM <epirat07@gmail.com> wrote:
> 
>> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>   doc/developer.texi | 11 +++++------
>>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>   @end example
>>>
>>>   @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an
>> else (to keep future patches cleaner)
>>>
>>
>> I agree with this, however people in the past pointed it out when new code
>> did this, IIRC.
>>
>> So if we merge this, people should stop flagging this in reviews, else its
>> frustrating
>> for new contributors following the documentation and then getting
>> contradicting reviews.
> 
> 
> would loooove a format defining script with a prehook that formats your
> patches before sending <3

That can be done automatically as one of the many jobs CI runs once we 
move to forgejo/gitlab. Same with every other check in patcheck.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1400 bytes --]

Hi

On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  doc/developer.texi | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >  @end example
> >
> >  @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >
> 
> I agree with this, however people in the past pointed it out when new code did this, IIRC.

yes, i know, ive seen it in both directions for this one.


> 
> So if we merge this, people should stop flagging this in reviews, else its frustrating
> for new contributors following the documentation and then getting contradicting reviews.

I think as long as its in a style guide, the author of a patch can easily
point the reviewer to the guide and that should resolve it immedeatly

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[epirat07]

On 28 Feb 2025, at 0:14, Michael Niedermayer wrote:

> Hi
>
> On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
>> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
>>
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>  doc/developer.texi | 11 +++++------
>>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>  @end example
>>>
>>>  @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>>>
>>
>> I agree with this, however people in the past pointed it out when new code did this, IIRC.
>
> yes, i know, ive seen it in both directions for this one.
>
>
>>
>> So if we merge this, people should stop flagging this in reviews, else its frustrating
>> for new contributors following the documentation and then getting contradicting reviews.
>
> I think as long as its in a style guide, the author of a patch can easily
> point the reviewer to the guide and that should resolve it immedeatly
>
> will apply
>

Sounds good.

Can you maybe check the styling issue at some point?
I had updated the CSS and locally I get the right classes assigned to the
codeblocks and the CSS styling them to hint which of them are good and which
bad examples.

But on the website deployed version its not working.

Thanks,
Marvin Scholz

> thx
>
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Devin Heitmueller]

On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > would loooove a format defining script with a prehook that formats your
> > patches before sending <3
>
> That can be done automatically as one of the many jobs CI runs once we
> move to forgejo/gitlab. Same with every other check in patcheck.

In my opinion, developers generally want to clean up style issues
prior to submission.  The problem with doing it in a CI job is that it
will just tell you after the fact that your patch didn't meet the
coding standards and you have to revise and resubmit.  The alternative
is for the CI job to "fix" the patch to conform, which people
generally don't want because what they submitted is different than
what gets committed.

For what it's worth, with the Linux Kernel we solved this years ago
with a "checkpatch.pl" which developers can run prior to committing.
Thus I could quickly run checkpatch.pl, fix anything it complains
about, and then run git commit and be confident that it conforms to
the standard.  Personally I prefer this approach as it lets me fix the
patches prior to submission, and the CI system isn't changing things
without my knowledge.

That said, I'm not necessarily against a CI job which validates it
meets the coding standards and kicks it back, to catch cases where the
developer failed to run checkpatch prior to submission.  This approach
only really works well though if the developer had an automated means
to run the check his/herself prior to submission.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1626 bytes --]

On Thu, Feb 27, 2025 at 07:40:14PM -0500, Devin Heitmueller wrote:
> On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > > would loooove a format defining script with a prehook that formats your
> > > patches before sending <3
> >
> > That can be done automatically as one of the many jobs CI runs once we
> > move to forgejo/gitlab. Same with every other check in patcheck.
> 
> In my opinion, developers generally want to clean up style issues
> prior to submission.  The problem with doing it in a CI job is that it
> will just tell you after the fact that your patch didn't meet the
> coding standards and you have to revise and resubmit.  The alternative
> is for the CI job to "fix" the patch to conform, which people
> generally don't want because what they submitted is different than
> what gets committed.
> 

> For what it's worth, with the Linux Kernel we solved this years ago
> with a "checkpatch.pl" which developers can run prior to committing.
> Thus I could quickly run checkpatch.pl, fix anything it complains
> about, and then run git commit and be confident that it conforms to
> the standard.  Personally I prefer this approach as it lets me fix the
> patches prior to submission, and the CI system isn't changing things
> without my knowledge.

we have tools/patcheck :)
I have the feeling we started forgeting about it

And I also think changing submissions in CI is problematic.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Devin
> Heitmueller
> Sent: Freitag, 28. Februar 2025 01:40
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule
> 
> On Thu, Feb 27, 2025 at 5:57 PM James Almer <jamrial@gmail.com> wrote:
> > > would loooove a format defining script with a prehook that formats
> your
> > > patches before sending <3
> >
> > That can be done automatically as one of the many jobs CI runs once we
> > move to forgejo/gitlab. Same with every other check in patcheck.
> 
> In my opinion, developers generally want to clean up style issues
> prior to submission.  The problem with doing it in a CI job is that it
> will just tell you after the fact that your patch didn't meet the
> coding standards and you have to revise and resubmit.  The alternative
> is for the CI job to "fix" the patch to conform, which people
> generally don't want because what they submitted is different than
> what gets committed.
> 
> For what it's worth, with the Linux Kernel we solved this years ago
> with a "checkpatch.pl" which developers can run prior to committing.
> Thus I could quickly run checkpatch.pl, fix anything it complains
> about, and then run git commit and be confident that it conforms to
> the standard.  Personally I prefer this approach as it lets me fix the
> patches prior to submission, and the CI system isn't changing things
> without my knowledge.
> 
> That said, I'm not necessarily against a CI job which validates it
> meets the coding standards and kicks it back, to catch cases where the
> developer failed to run checkpatch prior to submission.  This approach
> only really works well though if the developer had an automated means
> to run the check his/herself prior to submission.

Hi Devin,

I see it the same like you do, but even a bit more extreme: For me it must happen while writing. I don't leave anything improperly formatted excepting the current spot. Deferring formatting just causes trouble all the way in combination with version control. I don’t know how others are working, but I'm often making small commits which are pieced together later or commits with something that needs to go into a previous commit. Or sometimes commits need to be split and combined in a different way. When you are doing this this with a "format later" or "format sometimes only" approach, a mix of formatted/unformatted code lines will bite you all the time when rebasing/merging things around.
So, I prefer tooling that shows warnings and assists in formatting while typing. 
I have a manually adjusted profile, but it's just an approximate solution - it could be better.

What I had pitched a few years ago is the creation of a ruleset for a widely supported linter like Clang-tidy or similar which resembles the ffmpeg coding rules in detail. Then, everybody could use it in the way they prefer - be it "live" in a full-featured IDE or just by running manually.

sw








_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Lynne]

On 27/02/2025 02:10, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   doc/developer.texi | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a1bfe180c9b..6a753f99da6 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>   @end example
>   
>   @item
> -No braces around single-line blocks:
> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>   
>   @example c, good
>   // Good
> -if (bits_pixel == 24)
> +if (bits_pixel == 24) @{
>       avctx->pix_fmt = AV_PIX_FMT_BGR24;
> -else if (bits_pixel == 8)
> +@} else if (bits_pixel == 8) @{
>       avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> -else @{
> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> +@} else
>       return AVERROR_INVALIDDATA;
> -@}
> +
>   @end example
>   
>   @item

Strongly objecting to this patch. It's all over our codebase, and I like 
it. Literally everywhere you look.
If future patches need to add more under the branch, then it only needs 
to be done once.

Such fundamental changes need more than a single patch hastily accepted, 
and should involve the community as a whole, I believe.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2097 bytes --]

On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
> On 27/02/2025 02:10, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   doc/developer.texi | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index a1bfe180c9b..6a753f99da6 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >   @end example
> >   @item
> > -No braces around single-line blocks:
> > +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >   @example c, good
> >   // Good
> > -if (bits_pixel == 24)
> > +if (bits_pixel == 24) @{
> >       avctx->pix_fmt = AV_PIX_FMT_BGR24;
> > -else if (bits_pixel == 8)
> > +@} else if (bits_pixel == 8) @{
> >       avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> > -else @{
> > -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> > +@} else
> >       return AVERROR_INVALIDDATA;
> > -@}
> > +
> >   @end example
> >   @item
> 
> Strongly objecting to this patch. It's all over our codebase, and I like it.
> Literally everywhere you look.
> If future patches need to add more under the branch, then it only needs to
> be done once.
> 
> Such fundamental changes need more than a single patch hastily accepted, and
> should involve the community as a whole, I believe.

Its not a change, look at tools/patcheck

or try:

+    if(this)
+        that
+    else
+        nothat
+

patcheck reports:

missing } prior to else
patcheck.stdout:11:+    else

missing whitespace between keyword and ( (feel free to ignore)
patcheck.stdout:9:+    if(this)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If one takes all money from those who grow wealth and gives it to those who
do not grow wealth, 10 years later, almost the same people who where wealthy
will be wealthy again, the same people who where poor will be poor again.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2665 bytes --]

On Fri, Feb 28, 2025 at 12:25:43AM +0100, epirat07@gmail.com wrote:
> 
> 
> On 28 Feb 2025, at 0:14, Michael Niedermayer wrote:
> 
> > Hi
> >
> > On Thu, Feb 27, 2025 at 11:46:54PM +0100, epirat07@gmail.com wrote:
> >> On 27 Feb 2025, at 2:10, Michael Niedermayer wrote:
> >>
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>>  doc/developer.texi | 11 +++++------
> >>>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/doc/developer.texi b/doc/developer.texi
> >>> index a1bfe180c9b..6a753f99da6 100644
> >>> --- a/doc/developer.texi
> >>> +++ b/doc/developer.texi
> >>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >>>  @end example
> >>>
> >>>  @item
> >>> -No braces around single-line blocks:
> >>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
> >>>
> >>
> >> I agree with this, however people in the past pointed it out when new code did this, IIRC.
> >
> > yes, i know, ive seen it in both directions for this one.
> >
> >
> >>
> >> So if we merge this, people should stop flagging this in reviews, else its frustrating
> >> for new contributors following the documentation and then getting contradicting reviews.
> >
> > I think as long as its in a style guide, the author of a patch can easily
> > point the reviewer to the guide and that should resolve it immedeatly
> >
> > will apply
> >
> 
> Sounds good.
> 
> Can you maybe check the styling issue at some point?
> I had updated the CSS and locally I get the right classes assigned to the
> codeblocks and the CSS styling them to hint which of them are good and which
> bad examples.
> 
> But on the website deployed version its not working.

there seem to be 2 style.min.css
one in ffmpeg and one generated by Makefile in ffmpeg-web
but only one on the website, this may be slightly suboptimal

ive simply copied the one from ffmpeg over on the server now, backup in
style.min.css.bak2 if it breaks something and timo wants to undo

also that change, did change the look of the frontpage, we have a bigger
download button now and some empty space
between entries in "October 30th, 2016, Results: Summer Of Code 2016."

some html/css person, should look at this i think.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Nicolas George]

Michael Niedermayer (HE12025-02-28):
> we have tools/patcheck :)
> I have the feeling we started forgeting about it

You cannot click on it. Therefore, the coders who are somehow capable of
producing C code at quality level for FFmpeg but unable to use a command
line and unable to learn will not be able to use it and we should
definitely not cut ourselves from that promising contributor base.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Marvin S.]

On 28 Feb 2025, at 8:53, Nicolas George wrote:

> Michael Niedermayer (HE12025-02-28):
>> we have tools/patcheck :)
>> I have the feeling we started forgeting about it
>
> You cannot click on it. Therefore, the coders who are somehow capable of
> producing C code at quality level for FFmpeg but unable to use a command
> line and unable to learn will not be able to use it and we should
> definitely not cut ourselves from that promising contributor base.
>

I don't see how your snarky remarks here add anything valuable to the
discussion, please refrain from sending such pointless emails.

Thanks,
Marvin Scholz

> Regards,
>
> -- 
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Lynne]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 2294 bytes --]



On 28/02/2025 03:33, Michael Niedermayer wrote:
> On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
>> On 27/02/2025 02:10, Michael Niedermayer wrote:
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    doc/developer.texi | 11 +++++------
>>>    1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/doc/developer.texi b/doc/developer.texi
>>> index a1bfe180c9b..6a753f99da6 100644
>>> --- a/doc/developer.texi
>>> +++ b/doc/developer.texi
>>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
>>>    @end example
>>>    @item
>>> -No braces around single-line blocks:
>>> +No braces around single-line blocks, unless they are followed by an else (to keep future patches cleaner)
>>>    @example c, good
>>>    // Good
>>> -if (bits_pixel == 24)
>>> +if (bits_pixel == 24) @{
>>>        avctx->pix_fmt = AV_PIX_FMT_BGR24;
>>> -else if (bits_pixel == 8)
>>> +@} else if (bits_pixel == 8) @{
>>>        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
>>> -else @{
>>> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
>>> +@} else
>>>        return AVERROR_INVALIDDATA;
>>> -@}
>>> +
>>>    @end example
>>>    @item
>>
>> Strongly objecting to this patch. It's all over our codebase, and I like it.
>> Literally everywhere you look.
>> If future patches need to add more under the branch, then it only needs to
>> be done once.
>>
>> Such fundamental changes need more than a single patch hastily accepted, and
>> should involve the community as a whole, I believe.
> 
> Its not a change, look at tools/patcheck
> 
> or try:
> 
> +    if(this)
> +        that
> +    else
> +        nothat
> +
> 
> patcheck reports:
> 
> missing } prior to else
> patcheck.stdout:11:+    else
> 
> missing whitespace between keyword and ( (feel free to ignore)
> patcheck.stdout:9:+    if(this)

Ah, okay. I didn't read properly. The double negative in the sentence 
makes it hard to understand.

Rather than
 > No braces around single-line blocks, unless they are followed by an 
else (to keep future patches cleaner)

Consider
 > Don't wrap single-line blocks in braces. Use braces only if there is 
an accompanying else statement. This keeps future code changes easier to 
keep track of.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".