[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: docs/DeveloperPolicy.rst:395-408
+Commits with No Functional Change
+---------------------------------
+
+It may be permissible to commit changes without prior review when the changes
+have no semantic impact on the code if the changes being made are obvious and
+not invasive. For instance, removing trailing whitespace from a line, fixing a
+line ending to be consistent with the rest of the file, fixing a typo, code
----------------
chandlerc wrote:
> I think this is a much broader statement than is warranted to address the specific problem. And I'm not completely comfortable with it.
> 
> I don't think guidance around "no functional change" is the right way to give guidance about what is or isn't "obvious" and fine to commit with post-commit review personally.
> 
> I'd really suggest just being direct about *formatting* and whitespace changes, and specifically suggest that they not be made unless the file or code region in question is an area that the author is planning subsequent changes to.
We talk about formatting and whitespace in the CodingStandards document, but we talk about obviousness and post-commit review in DeveloperPolicy. Where would you like these new words to live? To me, they're more about the policy and less about the coding standard -- the coding standard says what the code should look like and the policy says what you should and should not do procedurally, but then I feel the need to tie it back to "obviousness". How about this in the developer policy:
```
The Obviousness of Formatting Changes
-------------------------------------

While formatting and whitespace changes may be "obvious", they can also create
needless churn that causes difficulties for out-of-tree users carrying local
patches. Do not commit formatting or whitespace changes unless they are related
to a file or code region that you intend to make subsequent changes to. The
formatting and whitespace changes should be highly localized, committed before
you begin functionality-changing work on the code region, and the commit message
should clearly state that the commit is not intended to change functionality,
usually by stating it is :ref:`NFC <nfc>`.


If you wish to make a change to formatting or whitespace that touches an entire
library or code base, please seek pre-commit approval first.
```


https://reviews.llvm.org/D50055