<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 22, 2015 at 6:15 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 22, 2015 at 2:47 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: alexfh<br>
Date: Mon Jun 22 04:47:44 2015<br>
New Revision: 240270<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D240270-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=MbOUt8HzIMl-_jRu7hLUERqQWUkAg6ETt9dGiGu7NDU&s=4Wl5VBbOhlLGd7wCCy-f6W2bslHxuiIWVvPE1djcw3g&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=240270&view=rev</a><br>
Log:<br>
Fixed/added namespace ending comments using clang-tidy. NFC<br></blockquote><div><br>Is this actually an LLVM or Clang coding convention? It doesn't seem to be & I'm not sure it's one worth adopting... might be worth some discussion before we go in this direction? (perhaps there has been discussion in a review thread, etc & I've missed it - helpful to call that out in the commit message (& probably put this in the LLVM coding conventions if we are going in that direction))<br></div></div></div></div></blockquote><div><br></div><div>I was having an impression that there's a convention to use namespace ending comments of this form, that's why I considered a purely mechanical change that doesn't need a discussion. However, now reading the <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_docs_CodingStandards.html-23namespace-2Dindentation&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=MbOUt8HzIMl-_jRu7hLUERqQWUkAg6ETt9dGiGu7NDU&s=MV-pGK58838khfDcGzIyOSsVlX0Q3pIqL1bq4BEaM2Y&e=">coding standards</a>, I see that the relevant rule is much weaker, it just allows adding comments, not even recommends them:</div><div><br></div></div></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div class="gmail_extra"><div class="gmail_quote"><div><span style="color:rgb(0,0,0);font-family:'Lucida Grande','Lucida Sans Unicode',Geneva,Verdana,sans-serif;font-size:14px;line-height:21px">If it helps readability, feel free to add a comment indicating what namespace is being closed by a </span><tt class="" style="font-family:Consolas,'Deja Vu Sans Mono','Bitstream Vera Sans Mono',monospace;font-size:0.95em;color:rgb(0,0,0);line-height:21px"><span class="">}</span></tt><span style="color:rgb(0,0,0);font-family:'Lucida Grande','Lucida Sans Unicode',Geneva,Verdana,sans-serif;font-size:14px;line-height:21px">.</span><br></div></div></div></blockquote> <div>and also the style of the comment used in the example is slightly different:<br></div><div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><pre style="font-family:Consolas,'Deja Vu Sans Mono','Bitstream Vera Sans Mono',monospace;font-size:0.95em;line-height:15.960000038147px;padding:0.5em;border:1px solid rgb(204,204,204);color:rgb(0,0,0);background-color:rgb(248,248,248)"><span class="" style="color:rgb(96,160,176);font-style:italic">} // end namespace llvm</span></pre></div></blockquote></div><div>instead of</div><div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><pre style="font-family:Consolas,'Deja Vu Sans Mono','Bitstream Vera Sans Mono',monospace;font-size:0.95em;line-height:15.960000038147px;padding:0.5em;border:1px solid rgb(204,204,204);color:rgb(0,0,0);background-color:rgb(248,248,248)"><span class="" style="color:rgb(96,160,176);font-style:italic">} // namespace llvm</span></pre></div></blockquote>used in this patch.</div><div><br></div><div>So an agreement on the style (and necessity) of the namespace ending comments prior to committing the patch wouldn't hurt. Would you like me to revert the patches before we figure out what the desired state is?</div><div><br></div><div>Also, what the preferred solution here would be?</div><div><br></div><div>  1. leave the comments (or the lack thereof) alone, don't even fix incorrect or inconsistent ones</div><div>  2. only fix incorrect comments</div><div>  3. fix incorrect comments and make all existing namespace ending comments consistent</div><div>  4. 3. + add namespace ending comments to all namespaces spanning more than X lines</div><div>  5. something else?</div><div><br></div><div>In cases 2-4 we need to decide which format of namespace ending comments to use:</div><div>  a. // llvm</div><div>  b. // namespace llvm</div><div>  c. // end namespace llvm</div><div>  d. // end of namespace llvm</div><div>  e. // end llvm namespace</div><div>  f. something else</div><div><br></div><div>This patch corresponds to 4b.</div></div>