<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, May 19, 2014 at 6:54 PM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Mon, May 19, 2014 at 6:53 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><div>On Mon, May 19, 2014 at 6:29 PM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@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">lg<br>
<br>
================<br>
Comment at: clang-tidy/llvm/NamespaceCommentCheck.cpp:66<br>
@@ +65,3 @@<br>
+ Token Tok;<br>
+ while (Lexer::getRawToken(Loc, Tok, Sources, Result.Context->getLangOpts())) {<br>
+ Loc = Loc.getLocWithOffset(1);<br>
----------------<br>
Can you add a comment explaining why it's ok to retry on failure with an incremented location?<br>
<br>
================<br>
Comment at: clang-tidy/llvm/NamespaceCommentCheck.cpp:73<br>
@@ +72,3 @@<br>
+ bool NextTokenIsOnSameLine = Sources.getSpellingLineNumber(Loc) == EndLine;<br>
+ bool NeedLineBreak = NextTokenIsOnSameLine && Tok.isNot(tok::eof);<br>
+<br>
----------------<br>
Can you explain why we need this? (I'd have expected the fix-it to not change the number of newlines, thus just inserting the comment between the } and the newline).<br></blockquote><div><br></div></div></div><div>Am I missing where you added a comment for this?</div>
</div></div></div></blockquote><div><br></div></div></div><div>Should be here: <a href="http://reviews.llvm.org/D3825?vs=9577&id=9579&whitespace=ignore-all#toc" target="_blank">http://reviews.llvm.org/D3825?vs=9577&id=9579&whitespace=ignore-all#toc</a> </div>
</div></div></div></blockquote><div><br></div><div>Ah, sorry, there's no comment for this. I've answered your question on the Phab only.</div><div>Should I add a comment?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div><br></div><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"><div class="gmail_extra">
<div class="gmail_quote">
<div> </div><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">
<br>
<a href="http://reviews.llvm.org/D3825" target="_blank">http://reviews.llvm.org/D3825</a><br>
<br>
<br>
</blockquote></div></div></div></blockquote></div><br>
</div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666"> </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> Google Germany, Munich</span></div>
</div></div>
</div></div>