[FFmpeg-devel] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[FFmpeg-devel] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

ffmpeg-git@ffmpeg.org (HE12025-08-12):
> Author:     Niklas Haas <git@haasn.dev>
> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> Commit:     Niklas Haas <ffmpeg@haasn.dev>
> CommitDate: Tue Aug 12 09:01:39 2025 +0000

Pushing after less than 24 hours without approval by the maintainer is
absolutely unacceptable, forge or no forge, and a spit in my face for
the time I invested into starting to look at the code, thankfully
limited.

Never do that again.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
> ffmpeg-git@ffmpeg.org (HE12025-08-12):
> > Author:     Niklas Haas <git@haasn.dev>
> > AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> > Commit:     Niklas Haas <ffmpeg@haasn.dev>
> > CommitDate: Tue Aug 12 09:01:39 2025 +0000
>
> Pushing after less than 24 hours without approval by the maintainer is
> absolutely unacceptable, forge or no forge, and a spit in my face for
> the time I invested into starting to look at the code, thankfully
> limited.

In https://ffmpeg.org/developer.html#Patch-review-process it says:

> After a patch is approved it will be committed to the repository.

I don't see anything about a time-frame requirement here.

That said, maybe we should adopt one, especially now that we actually
receive reviews in a timely manner thanks to the adoption of forgejo.

Perhaps we adopt the VLC model, which is:
- If a PR has received no activity for 3 days
- AND it has at least one approval
- AND it passes all CI checks
then it will be merged automatically, by a bot.

>
> Never do that again.
>
> --
>   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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:34:08 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> Perhaps we adopt the VLC model, which is:
> - If a PR has received no activity for 3 days
> - AND it has at least one approval
> - AND it passes all CI checks
> then it will be merged automatically, by a bot.

Oh, and perhaps obvious, but "it has no open discussion threads or change
requests" is of course another condition.
_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Gyan Doshi]

On 2025-08-12 03:04 pm, Niklas Haas wrote:
> On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
>> ffmpeg-git@ffmpeg.org (HE12025-08-12):
>>> Author:     Niklas Haas <git@haasn.dev>
>>> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
>>> Commit:     Niklas Haas <ffmpeg@haasn.dev>
>>> CommitDate: Tue Aug 12 09:01:39 2025 +0000
>> Pushing after less than 24 hours without approval by the maintainer is
>> absolutely unacceptable, forge or no forge, and a spit in my face for
>> the time I invested into starting to look at the code, thankfully
>> limited.
> In https://ffmpeg.org/developer.html#Patch-review-process it says:
>
>> After a patch is approved it will be committed to the repository.
> I don't see anything about a time-frame requirement here.

It's the last point in
https://ffmpeg.org/developer.html#Patches_002fCommitting

Regards,
Gyan

_______________________________________________
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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Niklas Haas (HE12025-08-12):
> I don't see anything about a time-frame requirement here.

For one, it does say something about a time-frame requirement.

For two, you have not waited for approval by maintainer.

> - AND it has at least one approval

Approval full stop is bullshit. The only approval worth something comes
from somebody who knows the code.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 15:08:10 +0530 Gyan Doshi <ffmpeg@gyani.pro> wrote:
>
>
> On 2025-08-12 03:04 pm, Niklas Haas wrote:
> > On Tue, 12 Aug 2025 11:16:19 +0200 Nicolas George <george@nsup.org> wrote:
> >> ffmpeg-git@ffmpeg.org (HE12025-08-12):
> >>> Author:     Niklas Haas <git@haasn.dev>
> >>> AuthorDate: Mon Aug 11 15:57:24 2025 +0200
> >>> Commit:     Niklas Haas <ffmpeg@haasn.dev>
> >>> CommitDate: Tue Aug 12 09:01:39 2025 +0000
> >> Pushing after less than 24 hours without approval by the maintainer is
> >> absolutely unacceptable, forge or no forge, and a spit in my face for
> >> the time I invested into starting to look at the code, thankfully
> >> limited.
> > In https://ffmpeg.org/developer.html#Patch-review-process it says:
> >
> >> After a patch is approved it will be committed to the repository.
> > I don't see anything about a time-frame requirement here.
>
> It's the last point in
> https://ffmpeg.org/developer.html#Patches_002fCommitting

I see that, but note:
1) the PR *was* approved
2) nobody is listed as a maintainer for these filters

Regardless, I still think we should then just establish a clear pipeline
here to prevent this sort of discussion from happening in the future.

Timo, can something like the VLC merge bot be implemented in ForgeJo?

>
> Regards,
> Gyan
>
> _______________________________________________
> 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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Nicolas George]

Niklas Haas (HE12025-08-12):
> I see that, but note:
> 1) the PR *was* approved

How do you know that approval was worth anything?

> 2) nobody is listed as a maintainer for these filters

So as a default, they are maintained by whoever maintains libavfilter as
a whole, which means me since Stefano took steps back from development.

> Regardless, I still think we should then just establish a clear pipeline
> here to prevent this sort of discussion from happening in the future.
> 
> Timo, can something like the VLC merge bot be implemented in ForgeJo?

Technical solution will not solve human problems.

-- 
  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] Pushing without approval avfilter/vf_vignette: use AVFilterContext for logging

[Niklas Haas]

On Tue, 12 Aug 2025 11:53:20 +0200 Nicolas George <george@nsup.org> wrote:
> Niklas Haas (HE12025-08-12):
> > I see that, but note:
> > 1) the PR *was* approved
>
> How do you know that approval was worth anything?

As far as my understanding goes, only people with write access to the
repository status have the ability to merge or approve PRs. If you do not
feel somebody can be trusted with write access, that is a separate issue
irrelevant to this discussion.

That aside, the person who reviewed the PR also has about 100x more activity
than you in the past 2-year period and is also a member of the GA. I think
it's fair to assume they are qualified to review a trivial patch.

> > 2) nobody is listed as a maintainer for these filters
>
> So as a default, they are maintained by whoever maintains libavfilter as
> a whole, which means me since Stefano took steps back from development.

Where in "MAINTAINERS" is this written?

> Technical solution will not solve human problems.

Having a clearly defined process is a human solution to a human problem.
_______________________________________________
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".