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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
