On Wed, Oct 19, 2016 at 8:27 AM, Benjamin Lerer <benjamin.le...@datastax.com > wrote:
> Having the test passing does not mean that a patch is fine. Which is why we > have a review check list. > I never put a patch available without having the tests passing but most of > my patches never pass on the first try. We always make mistakes no matter > how hard we try. > The reviewer job is to catch those mistakes by looking at the patch from > another angle. Of course, sometime, both of them fail. > Agreed. Review should not just be a "tests pass, +1" rubber stamp, but actually checking the code for correctness. The former is just process; the latter actually catches problems that the tests would not. (And this is true even if the tests are much much better than ours.) -- Jonathan Ellis co-founder, http://www.datastax.com @spyced