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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Regardless of the content of the patch, I think it should be reverted as it wasn't fully approved - more consensus is suitable for a patch like this & it wasn't @mehdi_amini's intent to sufficient approval alone.

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.

It sets the policy for new directories to be the newer one, not the LLVM/Clang one, which is a policy change so far as I can see.

If it were stated in the inverse - that those particular directories use the other convention, and "other directories" (including llvm/clang and any new directories) use the upper-start, that to me would be documenting existing practice, not a policy change.

In D140585#4028898 <https://reviews.llvm.org/D140585#4028898>, @aaron.ballman wrote:

> 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.

I think of these as bugs, rather than features - maybe we document them as existing bugs, or allow them as exceptions, but I think we should be striving for more consistency. The ability to move code between subprojects (eg: promoting some data structure to llvm's ADT library) would benefit from being smoother, and developers ability to move easily between subprojects without relearning conventions would be valuable.

Maybe we go ultimately end up with this patch/direction - that new projects use the new convention, and Clang/LLVM are left alone for now, maybe to be cleaned up/migrated later. I personally don't think the difference in naming is sufficiently valuable to justify this inconsistency (& I think it was a mistake to allow new subprojects with these deviating naming conventions - those should've been fixed before integrating them), and probably by LOC LLVM+Clang still dominate the project, so by LOC maybe we have more code using the old convention than the new, so a new subproject would have greater consistency across the project by using the old one (not to mention that, as you said, @aaron.ballman - the leaves are different from the core - most subprojects interact with LLVM, but not with each other - so for any given subproject, the code you interact with is probably dominated by LLVM and its naming convention).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140585