[PATCH] D140585: CodingStandards: restrict CamelCase variable names guideline to llvm/clang/clang-tools-extra/polly/bolt

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D140585#4027381 <https://reviews.llvm.org/D140585#4027381>, @MaskRay wrote:

> In D140585#4027224 <https://reviews.llvm.org/D140585#4027224>, @dblaikie wrote:
>
>> In D140585#4019532 <https://reviews.llvm.org/D140585#4019532>, @mehdi_amini wrote:
>>
>>> In D140585#4019417 <https://reviews.llvm.org/D140585#4019417>, @dblaikie wrote:
>>>
>>>> I'm not sure if @mehdi_amini meant their approval to be "this is good to commit in general" or "for their part, it looks OK, but other folks need buy in"?
>>>
>>> That’s how I meant it: sorry I should have left a comment about getting more approvals to reflect better consensus.
>>
>> @MaskRay  Could you please revert until this ^ is addressed?
>>
>> It doesn't look like https://discourse.llvm.org/t/top-level-clang-tidy-options-and-variablename-suggestion-on-codingstandards/58783 found/created more concensus that was (seems to be still is) missing from D108265 <https://reviews.llvm.org/D108265> that spawned that discourse thread.
>
> I am not sure about reverting this clarification patch. This patch isn't a policy change.
>
> In first few paragraphs there is a statement about naming convention.
> """
> Note that some code bases (e.g. ``libc++``) have special reasons to deviate
> from the coding standards.  For example, in the case of ``libc++``, this is
> because the naming and other conventions are dictated by the C++ standard.
>
> There are some conventions that are not uniformly followed in the code base
> (e.g. the naming convention). 
> """
>
> Previously, the description of variable names was clearly incorrect for all but llvm/clang/clang-tools-extra.

I think there is a somewhat deeper topic here. My initial understanding of the coding standards and related policies was that they're the standard for all LLVM projects. However, as you point out, they're really not, but those differences go beyond just coding style and into things like how build bots work, how code reviews work, documentation expectations, revert expectations, etc. LLVM, Clang and clang-tools-extra all generally follow the LLVM project guidelines while other projects such as libc++, lld, lldb, act more as separate-but-related projects with their own (sometimes undocumented) approaches. For example, our revert policy implies that it's reasonable for e.g., libc++ to revert a correct, conforming diagnostic change in Clang because it breaks a libc++ test which was testing the specific wording of that diagnostic. However, in practice, our preference is usually for the libc++ folks to fix the test rather than revert a correct, conforming change to Clang, but we don't really make this clear in our policies.

Personally, I think we want to give our subcommunities the autonomy needed to do what meets their needs, and that includes diverging from the primary documented policies. But then again, my mental model has changed somewhat about how I view projects in the monorepo. To me, Clang, LLVM, and clang-tools-extra are "upstream" tools while libc++, lldb, lld, etc are "downstreams" that consume the upstream tools. So to me, it would make sense for the developer policies apply to Clang, LLVM, and clang-tools-extra, while other projects are expected to document their deviations from the main policies. Because of that, I don't think we should do this by adding exceptions to the main policy (as done here) because that makes the main policies harder to understand. Instead, I'd rather we linked from the main policy back to various project policies (and vice versa) so it's more clear that there's a distinction between projects in the monorepo. This also makes it less contentious for these "downstream" projects to deviate from the main policies -- you don't have to convince the whole community the idea is good once the subcommunity has established a preference. Alternatively, we should get everyone onto the same policies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140585