[PATCH] D57896: Variable names rule

Michael Platings via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 18 08:51:25 PST 2019


michaelplatings marked an inline comment as done.
michaelplatings added inline comments.


================
Comment at: llvm/docs/CodingStandards.rst:1195
+  be camel case, and start with a lower case letter (e.g. ``leader`` or
+  ``boats``). It is also acceptable to use ``UpperCamelCase`` for consistency
+  with existing code.
----------------
rupprecht wrote:
> It would be nice for this section to be expanded a bit, just to avoid inevitable code review churn, e.g. if I'm adding 50 lines to a 200 line file, am I allowed to change the existing var names elsewhere in the file or method, or is that outside the scope of my change? If I'm reviewing that patch, do I tell the author they have to be consistent and revert other changes? etc.
> 
> Is there any plan to use clang-tidy to do a global cleanup, or is this going to be a totally ad-hoc migration -- variables use the new scheme only when the code is updated?
I've had a go at expanding it. Please let me know if you have other suggestions.

> Is there any plan to use clang-tidy to do a global cleanup, or is this going to be a totally ad-hoc migration -- variables use the new scheme only when the code is updated?

The latter. Given that the code doesn't keep to the existing .clang-tidy rules I'm not optimistic that we could persuade code owners to start now. That's not to say it couldn't happen eventually, but my aim at this point in time is to make it easier to use good variable names and I don't want perfect to be the enemy of better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57896/new/

https://reviews.llvm.org/D57896





More information about the llvm-commits mailing list