Re: [FFmpeg-devel] Revert "avformat/tls_openssl: properly get new BIO index"

[Alexander Strasser via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

On 2025-08-02 20:08 +0200, Nicolas George wrote:
> Alexander Strasser via ffmpeg-devel (HE12025-08-02):
> > I don't understand what the issue with the commit message is?
> 
> Two issues:

OK, now I understand what you mean.
It wasn't clear to me from your original replies in this thread.


> - Form: “>” is not part of our practice for commit message. Look at the
>   past year of commit message, the only ones that include “>” lines are
>   mistakes.

It is common to use this style of quoting outside of text MUAs as
well. E.g. when replying to things in IRC chat or in Markdown.

I find it acceptable and would encourage people to use it.

Not to paste emails by mistake, of course.
Though it is OK to quote mails on purpose if it helps to
have it included in the commit message. E.g.:

    From the discussion in the ML thread:
    > > This is like this and that.
    >
    > But there is also another case we need to consider.


Note that Kacper introduced the documentation quote like this (`^` for
highlighting inserted by me):

    type definition. As we can read in the documentation:
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    
    > Note that BIO_get_new_index() can only be used 127 times before it
    > returns an error.
    
    We cannot call it repeatedly, because it will fail eventually.
    
    To my understanding the index is not needed in our use and we could
    safely use BIO_TYPE_NONE. Documentation states:
                              ^^^^^^^^^^^^^^^^^^^^^
    
    > type can be set to either BIO_TYPE_NONE or via BIO_get_new_index() if
    > a unique type is required for searching (See BIO_find_type(3))

Would you agree that it is indeed a helpful way to use for quoting in
commit messages?


> - Substance: “To my understanding”, either the patch was reviewed, and
>   nobody's individual understanding has to be mentioned in the commit
>   message, or it was not and it should not have been applied.

I find it potentially helpful to include uncertainty in the commit
message. It is helpful for people doing code archaeology.


> It is minor, but there is a reason we historically gave write access
> only to experienced contributors: two pairs of eyes are better than one
> to fix this kind of minor mistakes and keep our history as clean as
> possible.

This was a special case, the commit that changed that code recently
introduced a regression. Therefore reverting earlier is better.

As soon as someone has a better understanding, the code can be changed
again or left as is. No reason to let the regression live longer just
to avoid reverts. These kinds of reverts are healthy as long as they
include the reason why the reverts were done.

I agree with you that we should try to keep our history as clean as
possible and so does Kacper as he already stated.


  Alexander
_______________________________________________
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".