[FFmpeg-devel] [RFC] Reviewing merges

James Almer jamrial at gmail.com
Mon Apr 24 17:23:16 EEST 2017


On 4/23/2017 11:07 PM, Michael Niedermayer wrote:
> Hi all
> 
> Should changes ported from libav (what we call merges) be reviewed
> before being pushed?

The lot of merges are simple things like "Fix this bug that was already
fixed in ffmpeg months ago", "K&R", etc. Lately we are even no-oping a
good amount of them as they don't even apply.
Only a small set of merges are big API changes, and those are always
handled by more than one developer, either by reviewing the changes,
testing them or by helping getting the thing working. And of course, any
bug that was not caught before pushing is fixed afterwards.
In fact, it should be noted that if they are initially skipped for
requiring more thought and for blocking unrelated merges, when we get
them working at a latter point we always send them to the ML instead of
simply pushing them.

Back when you handled the merges you were able to keep things synced
daily since you were not slowed down by the need of having a review
process for every single one of them, and similarly only asked for help
with big changes that required every module to be adapted, or for
opinions with considerable changes like ref counting, much like Clément
asked for opinions about the new bitstream reader.

We have recently been able to go through six hundred or so commits in a
month or two this way after being stuck for the longest time by a few of
those big API changes. If we start requiring every commit to go through
a review process on the ML then we will never catch up with the backlog.
In short, things as they are right now are smooth. Changing it will only
make this slower.

That aside, may i ask what prompted this question? Did someone complain
to you privately? No merge recently seems to have broken anything that
hasn't been already fixed, beyond one threading bug that's being
investigated right now, so i wonder why ask this now.


More information about the ffmpeg-devel mailing list