[llvm] [llvm][Docs] Explain how to handle excessive formatting changes (PR #126239)

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 7 04:34:21 PST 2025


https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/126239

Based on some feedback in Discord about a PR where a reviewer asked the author to move the formatting changes to a new PR, which appears to contradict the current form of this document.

I've added an explanation here, before the point where the author would be committing any of the formatting changes.

There are other ways this can go, for example some projects don't want the churn of formatting, or you can pre-emptively send a formatting PR, but I don't think enumerating them all here will help the audience for this text.

So I've recomended one path that will start them off well, and can branch off if the reviewers make requests.

>From 0568843ab5c6e02636bb7b5450fd8a3165071af4 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 7 Feb 2025 12:05:38 +0000
Subject: [PATCH] [llvm][Docs] Explain how to handle excessive formatting
 changes

Based on some feedback in Discord about a PR where a reviewer
asked the author to move the formatting changes to a new PR,
which appears to contradict the current form of this document.

I've added an explanation here, before the point where the author
would be committing any of the formatting changes.

There are other ways this can go, for example some projects don't
want the churn of formatting, or you can pre-emptively send a
formatting PR, but I don't think enumerating them all here will
help the audience for this text.

So I've recomended one path that will start them off well,
and can branch off if the reviewers make requests.
---
 llvm/docs/Contributing.rst | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/llvm/docs/Contributing.rst b/llvm/docs/Contributing.rst
index 9311f39b6e6978d..b34b680854e77ed 100644
--- a/llvm/docs/Contributing.rst
+++ b/llvm/docs/Contributing.rst
@@ -73,15 +73,33 @@ recent commit:
 
   % git clang-format HEAD~1
 
-Note that this modifies the files, but doesn't commit them -- you'll likely want
-to run
+.. note::
+  For some patches, formatting them may add changes that obscure the intent of
+  the patch. For example, adding to an enum that was not previously formatted
+  may result in the entire enum being reformatted. This happens because not all
+  of the LLVM Project conforms to clang-format at this time.
+
+  If you think that this might be the case for your changes, or are unsure, we
+  recommend that you add the formatting changes as a **separate commit** within
+  the Pull Request.
+
+  Reviewers may request that this formatting commit be made into a separate Pull
+  Request that will be merged before your actual changes.
+
+  This means that if the formatting changes are the first commit, you will have
+  an easier time doing this. If they are not, that is ok too, but you will have
+  to do a bit more work to separate it out.
+
+Note that ``git clang-format`` modifies the files, but doesn't commit them -- you'll likely want
+to run one of the following to add the changes to a commit:
 
 .. code-block:: console
 
+  # To create a new commit.
+  % git commit -a
+  # To add to the most recent commit.
   % git commit --amend -a
 
-in order to update the last commit with all pending changes.
-
 .. note::
   If you don't already have ``clang-format`` or ``git clang-format`` installed
   on your system, the ``clang-format`` binary will be built alongside clang, and



More information about the llvm-commits mailing list