[llvm-dev] [docs][RFC] Style for "end namespace" comments

[James Y Knight via llvm-dev]

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/95fecc8b/attachment.html>