[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. (at least if/when D140585 sticks, then the format should be reflected at the top level, not omitted/left to subprojects to specify - then/at the same time, the explicit naming convention specified in mlir, compiler-rt, etc, should be

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. My objections to this initially still seem to stand - the discourse thread doesn't appear to have shown more consensus & I still don't think we should remove a top level naming convention entirely. We should have a convention documented and specified in the top level

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. I'll wait a bit and land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

Amir accepted this revision. Amir added a comment. This revision is now accepted and ready to land. LGTM for BOLT Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. Created D140585 to update `llvm/docs/CodingStandards.rst`. Changed the summary to mention https://discourse.llvm.org/t/top-level-clang-tidy-options-and-variablename-suggestion-on-codingstandards/58783 Repository: rG LLVM Github

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay updated this revision to Diff 484955. MaskRay edited the summary of this revision. MaskRay added a comment. Herald added a reviewer: bollu. Herald added subscribers: yota9, ayermolo, StephenFan. Herald added a reviewer: rafauler. Herald added a reviewer: Amir. Herald added a reviewer:

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

mehdi_amini added a comment. > the original discussion that spawned the (not-yet-accepted, as it says in its > opening sentence) linked proposal around variable naming. +1 with David again: if the proposal gets accepted, then this patch makes sense to me. Repository: rG LLVM Github

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. Raised https://lists.llvm.org/pipermail/llvm-dev/2021-August/152210.html ("Top-level .clang-tidy options and VariableName suggestion on CodingStandards") Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. The current state of the root `.clang-tidy` accurately reflects the LLVM Coding Conventions as documented, which applies to LLVM and its subprojects (some subprojects have diverged from this standard). The place for a discussion to change the naming convention is not

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. In D108265#2953305 , @dblaikie wrote: > In D108265#2952555 , @MaskRay wrote: > >> The number of top-level projects using `VariableName` is smaller than the >> number of projects not

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. In D108265#2952555 , @MaskRay wrote: > The number of top-level projects using `VariableName` is smaller than the > number of projects not using the style. > The top-level variable style just provoked projects to either override

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. The number of top-level projects using `VariableName` is smaller than the number of projects not using the style. The top-level variable style just provoked projects to either override the options (flang/, lld/, mlir/) or disable the check. `VariableName` is not even a

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

mehdi_amini added a comment. +1 on arguments from @dblaikie : I'm not sure I understand the motivation for this change right now? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay added a comment. In D108265#2951222 , @dblaikie wrote: > I think it should probably stay as-is, given this is the documented LLVM > project naming convention: >

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. (eg: new projects should benefit from the LLVM default- less chance of further diversification of naming conventions) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108265/new/ https://reviews.llvm.org/D108265

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

dblaikie added a comment. I think it should probably stay as-is, given this is the documented LLVM project naming convention: https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly - a bit of extra friction for subprojects to opt-out of that

[PATCH] D108265: .clang-tidy: Push variable related readability-identifier-naming options down to projects

MaskRay created this revision. MaskRay added reviewers: dblaikie, mehdi_amini. MaskRay requested review of this revision. Herald added subscribers: cfe-commits, llvm-commits, aheejin. Herald added projects: LLVM, clang-tools-extra. The variable related top-level readability-identifier-naming