Thanks :) On 5/19/19 7:00 PM, Devin Heitmueller wrote: > 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 > _______________________________________________ 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".