Hello Paul, On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <one...@gmail.com> wrote: > > On 5/17/19, Mathieu Duponchelle <math...@centricular.com> wrote: > > There isn't one, as I said the added indentation is because of the new loop! > > To get this committed to tree you need to comply to review requests.
I think Mathieu's point is that the code indentation change was not cosmetic - it's because the code in question is now inside a for loop, and thus it needed to be indented another level. Are you suggesting he should make a patch which results in the indentation being wrong, and then submit a second patch which fixes the incorrect indentation introduced by the first patch? I'm confident that Mathieu is trying to comply with review requests so he can get his code merged. In this particular case, Carl raised his concern about the indentation, Mathieu responded by suggesting that given there was a functional change the re-indent was correct, and then there was silence (i.e. neither agreement nor disagreement). I'm also asking because I have work which I'm hoping to get upstreamed as well at some point, and I'm sure I've got similar things in my patches. And having spent 20+ years reviewing patches on lots of other open source projects, I wouldn't have thought twice about accepting the patch as-is (given the change in indentation is a result of a functional change and is not cosmetic). In my experience, this particular patch isn't an example of what maintainers mean when they say "don't mix cosmetic whitespace changes with functional changes". Regards, Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com _______________________________________________ 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".
