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

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211207/c928d2fe/attachment-0001.html>

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211207/6edd0444/attachment.html>