<div dir="ltr">Ahmed, first please don't build up such a large bank of commits and push them all at once.<div><br></div><div>This first causes significant load on the reviewers that is hard to predict and absorb. Secondly, it makes build bots useless at tell you which commit broke something and allowing fine grained rollbacks.</div>
<div><br></div><div>Please mail and commit a patch as soon as it is ready / approved.</div><div><br></div><div><br></div><div>Second, while this patch is clearly trivial, and I saw that the first last of this long chain of patches introduced some testing facilities, *please* introduce the test harness *first*, and then have incremental tests attached to each commit so reviewers can see what has been fixed.</div>
<div><br></div><div>Finally, it doesn't look like any of this series really got reviewed before committing. The last review I see with anything to do with MC CFG construction was back in May, and you seemed to think pre-commit review was appropriate there, so I don't know what some of these (not this one, I just really don't want to write you N different emails with feedback) didn't need review first.</div>
<div><br></div><div>-Chandler</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 21, 2013 at 12:27 AM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ab<br>
Date: Wed Aug 21 02:27:44 2013<br>
New Revision: 188872<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=188872&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=188872&view=rev</a><br>
Log:<br>
MC CFG: uint64_t -> size_t for vector size.<br>
<br>
Modified:<br>
    llvm/trunk/include/llvm/MC/MCAtom.h<br>
<br>
Modified: llvm/trunk/include/llvm/MC/MCAtom.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAtom.h?rev=188872&r1=188871&r2=188872&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAtom.h?rev=188872&r1=188871&r2=188872&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/include/llvm/MC/MCAtom.h (original)<br>
+++ llvm/trunk/include/llvm/MC/MCAtom.h Wed Aug 21 02:27:44 2013<br>
@@ -139,7 +139,7 @@ public:<br>
<br>
   const MCDecodedInst &back() const { return Insts.back(); }<br>
   const MCDecodedInst &at(size_t n) const { return Insts.at(n); }<br>
-  uint64_t size() const { return Insts.size(); }<br>
+  size_t size() const { return Insts.size(); }<br>
   /// @}<br>
<br>
   /// \name Atom type specific split/truncate logic.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>