2014-08-11 11:20 GMT-07:00 Chandler Carruth <[email protected]>: > > On Mon, Aug 11, 2014 at 11:09 AM, Alex L <[email protected]> wrote: > >> Sorry, let me try to explain. >> I've commited a patch with the tests for coverage mapping on Friday with >> all the tests that failed on some buildbots and all of the windows >> buildbots. >> > > OK > > >> I've resubmitted the patch today with all the tests, but 2 of the tests >> need this commit to pass on windows. >> > > 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. >
Thanks, this would probably be a better way to approach this. > > >> 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 >> are required for the generation. >> > > 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. > > 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. > > 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.) > I will do that then - revert this commit and split the test patch. Cheers
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
