[llvm-dev] [docs][RFC] Style for "end namespace" comments
Carlos Galvez via llvm-dev
llvm-dev at lists.llvm.org
Tue Dec 7 05:39:57 PST 2021
Pushed now, thanks everyone for your time!
On Tue, Dec 7, 2021 at 2:30 PM James Y Knight <jyknight at google.com> wrote:
> I accepted the revision. Please submit, so we can put this issue to rest.
> :)
>
>
> On Tue, Dec 7, 2021 at 6:52 AM Carlos Galvez via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> > especially if clang-tidy does the same job.
>> Thanks, I forgot clang-tidy also enforces this. I've played with it and
>> it will change the existing style <https://godbolt.org/z/z6d5KnhaG> if
>> you have a typo on the namespace.
>>
>> > This is one of those things that aren't worth having a patch just for
>> this.
>> Fully agree, I meant as part of the pre-commit "git clang-format" that we
>> anyway do on regular patches. But I can see how even that can cause
>> significant noise during review.
>>
>> So to summarize the comments so far:
>>
>> - Most predominant style is "// namespace".
>> - Style in Coding Guidelines should be consistent with clang-format
>> and clang-tidy.
>> - Regardless of what it says in the Coding Guidelines, both styles
>> are OK and achieve the same goal -> reviewers should not request changes
>> here.
>> - It's not worth changing existing code.
>>
>> Based on that, would you be OK with proceeding with the patch
>> <https://reviews.llvm.org/D115115> to update the examples in the Coding
>> Guidelines? If not, do you have any suggestions on what should be improved,
>> or should I just drop it altogether?
>>
>> Best regards,
>> Carlos
>>
>>
>>
>> On Tue, Dec 7, 2021 at 11:50 AM Renato Golin <rengolin at gmail.com> wrote:
>>
>>> On Tue, 7 Dec 2021 at 10:07, Carlos Galvez via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>>> I personally thing this is just something clang-format should update
>>>> automatically as files get modified - over time the style automatically
>>>> becomes consistent.
>>>>
>>>
>>> No, clang-format shouldn't be changing the actual contents as much as it
>>> can, especially if clang-tidy does the same job.
>>>
>>> IIUC, James proposal was to match the documentation with whatever
>>> clang-tidy emits, whichever way it goes.
>>>
>>> The meaning of all versions are exactly the same and it doesn't really
>>> matter which way you have it.
>>>
>>> I echo some comments here that we should focus on bigger things.
>>>
>>> The only action I would take here is to update either the docs, or
>>> clang-tidy, to match each other and the general current preference, which
>>> seems to be "// namespace" (the blue bars).
>>>
>>> I would strongly discourage people patching existing sources to match
>>> the style just for the sake of style. This is one of those things that
>>> aren't worth having a patch just for this.
>>>
>>> cheers,
>>> --renato
>>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211207/ec191963/attachment.html>
More information about the llvm-dev
mailing list