We don’t have time for code reviews

#1

Post by javinpaul » Wed, Oct 23, 2013, 1:23 PM UTC

We don’t have time for code reviews
blog.8thcolor.com

Re: We don’t have time for code reviews

#2

Post by alphadevx » Wed, Oct 23, 2013, 1:42 PM UTC

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

Re: We don’t have time for code reviews

#3

Post by mabbo » Wed, Oct 23, 2013, 1:42 PM UTC

I once pushed un-reviewed code that contained

  alert("Fuck");

It made it to a single user who was testing it for me, on the other side of the planet. He spoke very little English, and for that I am grateful.

That was my last push without review.

Re: We don’t have time for code reviews

#4

Post by eloisant » Wed, Oct 23, 2013, 1:46 PM UTC

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

Re: We don’t have time for code reviews

#5

Post by martinvanaken » Wed, Oct 23, 2013, 1:51 PM UTC

eloisant wrote: post #4

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

Hi, I think both are actually useful. The CI is supposed to run the tests (even if I like to run some myself when reviewing), but I agree that starting the application is a part of the review - you should not stop at just looking at the code (but just startint the application does not cut it either for me).

Martin (OP)

Re: We don’t have time for code reviews

#6

Post by alphadevx » Wed, Oct 23, 2013, 1:51 PM UTC

eloisant wrote: post #4

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

In my team each feature branch is run by our build server (unit test), even before merge.

Re: We don’t have time for code reviews

#7

Post by benihana » Wed, Oct 23, 2013, 1:51 PM UTC

eloisant wrote: post #4

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

Code reviews are about psychology. When you know your teammates are going to be reviewing your code, you write it differently than you do when you know no one but you will ever look at it.

Static analysis and regression tests are tools to make sure the code isn't broken.

Re: We don’t have time for code reviews

#8

Post by martinvanaken » Wed, Oct 23, 2013, 1:52 PM UTC

mabbo wrote: post #3

I once pushed un-reviewed code that contained alert("Fuck"); It made it to a single user who was testing it for me, on the other side of the planet. He spoke very little English, and for that I am grateful. That was my last push without review.

This one should be recorded on http://www.codingconfessional.com/

Re: We don’t have time for code reviews

#9

Post by warp » Wed, Oct 23, 2013, 1:52 PM UTC

eloisant wrote: post #4

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

And yet most shops don't even do that :(

Re: We don’t have time for code reviews

#10

Post by masklinn » Wed, Oct 23, 2013, 1:53 PM UTC

eloisant wrote: post #4

alphadevx wrote: post #2

Agree with this: pull requests are code reviews. Web UIs with visual diffs make this even more efficient.

I don't know, code review without running the code is a very very light variant, too light to call it code review I believe.

Code review is about reviewing the code. That's literally what the name says. CI runs the tests, QA test the final behavior, code review checks the code.