<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2014-08-11 11:20 GMT-07:00 Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span>:<br>
<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"><br><div class="gmail_quote"><div class="">On Mon, Aug 11, 2014 at 11:09 AM, Alex L <span dir="ltr"><<a href="mailto:arphaman@gmail.com" target="_blank">arphaman@gmail.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">Sorry, let me try to explain.<div>I've commited a patch with the tests for coverage mapping <span><span>on Friday</span></span> with all the tests that failed on some buildbots and all of the windows buildbots.</div>

</div></blockquote><div><br></div></div><div>OK</div><div class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I've resubmitted the patch today with all the tests, but 2 of the tests need this commit to pass on windows.</div>

</div></blockquote><div><br></div></div><div>It would have made more sense to have a single commit with this functionality and those two tests. =/ The idea of incremental development is that you commit the minimum chunk of functionality and the tests for that functionality in a single commit.</div>
</div></div></div></blockquote><div><br></div><div>Thanks, this would probably be a better way to approach this.</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 class="">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div>All the tests are batched in a single patch because the code for coverage mapping generation was commited before the tests, and all of those tests</div><div>are required for the generation.</div></div></blockquote></div>
</div>
<br>Ok, please don't submit patches this way in the future. I'm sorry that someone OK'ed the patch with the coverage mapping generation to be committed, they were wrong to do so. We do not accept patches that add functionality without tests, period.</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">If the code review for the tests is finished at this point, please commit them with an "oops" and a pointer to the commit(s) with the functionality they are testing.</div>

<div class="gmail_extra"><br></div><div class="gmail_extra">However, if the tests are not ready to be committed (that is, the code review is not finished) then I would suggest reverting your functionality and merging it into the patch which contains the test cases for that functionality. (And potentially splitting that into smaller chunks for easier review.)</div>
</div></blockquote><div><br></div><div>I will do that then - revert this commit and split the test patch.</div><div>Cheers </div></div><br></div></div>