Quoting Rémi Denis-Courmont (2023-08-25 16:22:45) > Le torstaina 24. elokuuta 2023, 22.56.14 EEST Michael Niedermayer a écrit : > > Suggested text is from Anton > > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > --- > > doc/developer.texi | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/doc/developer.texi b/doc/developer.texi > > index 0c2f2cd7d1..383120daaa 100644 > > --- a/doc/developer.texi > > +++ b/doc/developer.texi > > @@ -853,6 +853,9 @@ Everyone is welcome to review patches. Also if you are > > waiting for your patch to be reviewed, please consider helping to review > > other patches, that is a great way to get everyone's patches reviewed > > sooner. > > > > +Reviews must be constructive > > Well, frankly, no. You're really confusing code reviews with teaching here. A > code review is first and foremost meant to find problems and avoid adding > bugs > or bad designs into the code base. It is not meant to solve problems. Of > course, it is preferable for a review to be constructive, but it is not > always > possible or reasonable. > > Sometimes code just does not belong in. > > Sometimes the reviewer can prove or make strong arguments that a patch is > broken or bad, without having constructive feedback to give. This is just > like > mathematical proofs. Some are constructive, some aren't. > > 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. The intent is rather that every review (other than OK) should be based on technical arguments and not be a semantic equivalent of 'no'. In case you did not notice, we have a persistent problem with some people who are sending "reviews" of exactly this type. I don't think that should be acceptable. Would you be happier with some reformulation of the text that makes this more clear? -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".