PR #22344 opened by Zhao Zhili (quink) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22344 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22344.patch
The cosmetic-changes policy in developer.texi was written during the SVN era, when reviewing indentation changes mixed with functional changes was genuinely difficult. Since FFmpeg has moved to Git, reviewers now have simple built-in tools to ignore whitespace changes: git diff -w git log -p --ignore-all-space Forgejo's pull request UI also offers a 'Hide whitespace changes' toggle, making it trivial to focus on the functional diff. For those who prefer reviewing patches in their mail client, the same result can be achieved by saving the patch and running: git apply --ignore-whitespace <patch> && git diff -w Relax the policy so that indentation changes which are invisible to git diff --ignore-all-space may accompany functional changes, while still requiring non-whitespace cosmetic changes to be in separate commits. Signed-off-by: Zhao Zhili <[email protected]> >From 7f5ccefabab8cb0db020963fd67a29b182c060f3 Mon Sep 17 00:00:00 2001 From: Zhao Zhili <[email protected]> Date: Mon, 2 Mar 2026 10:42:05 +0800 Subject: [PATCH] doc/developer: allow whitespace changes mixed with functional changes The cosmetic-changes policy in developer.texi was written during the SVN era, when reviewing indentation changes mixed with functional changes was genuinely difficult. Since FFmpeg has moved to Git, reviewers now have simple built-in tools to ignore whitespace changes: git diff -w git log -p --ignore-all-space Forgejo's pull request UI also offers a 'Hide whitespace changes' toggle, making it trivial to focus on the functional diff. For those who prefer reviewing patches in their mail client, the same result can be achieved by saving the patch and running: git apply --ignore-whitespace <patch> && git diff -w Relax the policy so that indentation changes which are invisible to git diff --ignore-all-space may accompany functional changes, while still requiring non-whitespace cosmetic changes to be in separate commits. Signed-off-by: Zhao Zhili <[email protected]> --- doc/developer.texi | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/doc/developer.texi b/doc/developer.texi index f1c8e42840..a78383417c 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -468,13 +468,18 @@ ask/discuss it on the developer mailing list. @subheading Cosmetic changes should be kept in separate patches. We refuse source indentation and other cosmetic changes if they are mixed -with functional changes, such commits will be rejected and removed. Every +with functional changes, such commits will be rejected and removed. However, +indentation changes that can be ignored by @code{git diff --ignore-all-space} +(e.g. changes in whitespace amount, leading/trailing spaces) may be mixed with +functional changes, since reviewers can use @code{git diff -w} or +@code{git log -p --ignore-all-space} to review only the functional parts of +the change. Forgejo's pull request interface also provides a +``Hide whitespace changes'' option for this purpose. Every developer has his own indentation style, you should not change it. Of course if you (re)write something, you can use your own style, even though we would prefer if the indentation throughout FFmpeg was consistent (Many projects force a given indentation style - we do not.). If you really need to make -indentation changes (try to avoid this), separate them strictly from real -changes. +non-whitespace cosmetic changes, separate them strictly from real changes. NOTE: If you had to put if()@{ .. @} over a large (> 5 lines) chunk of code, then either do NOT change the indentation of the inner part within (do not -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
