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

[Chris Lattner via llvm-dev]

Thank you for driving this Carlos!

-Chris

> On Dec 7, 2021, at 5:39 AM, Carlos Galvez via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Pushed now, thanks everyone for your time!
> 
> On Tue, Dec 7, 2021 at 2:30 PM James Y Knight <jyknight at google.com <mailto: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 <mailto: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 <mailto:rengolin at gmail.com>> wrote:
> On Tue, 7 Dec 2021 at 10:07, Carlos Galvez via llvm-dev <llvm-dev at lists.llvm.org <mailto: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 <mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> _______________________________________________
> 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/20211210/e518c6bb/attachment.html>