
<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Thank you for driving this Carlos!<div class=""><br class=""></div><div class="">-Chris<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Dec 7, 2021, at 5:39 AM, Carlos Galvez via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Pushed now, thanks everyone for your time!</div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 7, 2021 at 2:30 PM James Y Knight <<a href="mailto:jyknight@google.com" class="">jyknight@google.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="">I accepted the revision. Please submit, so we can put this issue to rest. :)<div class=""><br class=""></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 7, 2021 at 6:52 AM Carlos Galvez via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="">> especially if clang-tidy does the same job.<div class="">Thanks, I forgot clang-tidy also enforces this. I've played with it and it will <a href="https://godbolt.org/z/z6d5KnhaG" target="_blank" class="">change the existing style</a> if you have a typo on the namespace.</div><div class=""><br class=""></div><div class="">> This is one of those things that aren't worth having a patch just for this.</div><div class="">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.</div><div class=""><br class=""></div><div class="">So to summarize the comments so far:</div><div class=""><ul class=""><li class="">Most predominant style is "// namespace".</li><li class="">Style in Coding Guidelines should be consistent with clang-format and clang-tidy.</li><li class="">Regardless of what it says in the Coding Guidelines, both styles are OK and achieve the same goal -> reviewers should not request changes here.</li><li class="">It's not worth changing existing code.<br class=""></li></ul><div class="">Based on that, would you be OK with proceeding with the <a href="https://reviews.llvm.org/D115115" target="_blank" class="">patch</a> 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?</div><div class=""><br class=""></div><div class="">Best regards,</div><div class="">Carlos</div><div class=""><br class=""></div></div><div class=""><br class=""></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 7, 2021 at 11:50 AM Renato Golin <<a href="mailto:rengolin@gmail.com" target="_blank" class="">rengolin@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div dir="ltr" class="">On Tue, 7 Dec 2021 at 10:07, Carlos Galvez via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class=""></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class=""><div class="">I personally thing this is just something clang-format should update automatically as files get modified - over time the style automatically becomes consistent.</div></div></blockquote><div class=""><br class=""></div><div class="">No, clang-format shouldn't be changing the actual contents as much as it can, especially if clang-tidy does the same job.</div><div class=""><br class=""></div><div class="">IIUC, James proposal was to match the documentation with whatever clang-tidy emits, whichever way it goes. </div><div class=""><br class=""></div><div class="">The meaning of all versions are exactly the same and it doesn't really matter which way you have it.</div><div class=""><br class=""></div><div class="">I echo some comments here that we should focus on bigger things.</div><div class=""><br class=""></div><div class="">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).</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">cheers,</div><div class="">--renato</div></div></div>

</blockquote></div>

_______________________________________________<br class="">

LLVM Developers mailing list<br class="">

<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a><br class="">

<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br class="">

</blockquote></div>

</blockquote></div>

_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<br class=""></div></blockquote></div><br class=""></div></body></html>