Unless the reviewer reviews the for content, then you don’t know if they do or not.
-Jeremiah > On Oct 19, 2016, at 10:52 AM, Jonathan Haddad <j...@jonhaddad.com> wrote: > > Shouldn't the tests test the code for correctness? > > On Wed, Oct 19, 2016 at 8:34 AM Jonathan Ellis <jbel...@gmail.com> wrote: > >> 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 >>
