[FFmpeg-devel] [PATCH 1/2] doc/developer: Reviews must be constructive

[Anton Khirnov]

Quoting Rémi Denis-Courmont (2023-08-25 17:09:55)
> Le perjantaina 25. elokuuta 2023, 17.58.40 EEST Anton Khirnov a écrit :
> > > And then sometimes an argument has been argued to death previously and
> > > there is really no point to rehash it again and again. If people cannot
> > > agree, they should refer to the TC, not brute force the review through
> > > overwhelming insistance.
> > 
> > I think we just have different interpretations of the word
> > 'constructive' here.
> > I certainly agree that some patches are just not acceptable - I certainly
> > did not mean to imply that there must be a way forward for all patches.
> 
> I think that you do not agree with the generally accepted meaning of 
> "constructive" in this context. By definition a review cannot be constructive, 
> as in helpful or conducive of a way forward, if it argues that there are no 
> ways forward.

Explaining why a patch is not acceptable is helpful IMO.
Saying 'no', on the other hand, is not.

> Maybe you meant "supported" or "corroborated".

Might as well describe it in more than one word, since apparently it's
so unclear. Would you be in favor of something along the lines of

  Nontrivial (i.e. other than cosmetics or accepting the patch) reviews
  must be based on technical arguments. If the reviewer fails to provide
  arguments for rejecting the patch or requesting changes, then the
  review may be disregarded.

-- 
Anton Khirnov