Live data from Hacker News

We don’t have time for code reviews

blog.8thcolor.com

31–40 of 67 posts

Re: We don’t have time for code reviews

#32

You think that's bad, I once worked in a startup where the Product Manager told me he didn't have time for tests.

You speak as if that's unusual...

If it's not something you can stick on an invoice, it's generally hard to persuade management it's worth doing in my experience.

Re: We don’t have time for code reviews

#33
post #28

Pull requests are insufficient for code review. You need to run the code. I often find the code I want to review after following a path in a debugger.

I think I have to disagree with this one.

A major part of being an effective programmer is the ability to maintain state and call stacks in your head.

Stepping through code is great - as a last resort. But I've found that if I need to run through it with a debugger in order to understand it, it is often overly complex.

As with any generalized rule there are exceptions, but I consider needing to step through code in a debugger to understand its behavior as that exception, not the inverse.

Re: We don’t have time for code reviews

#34
When talking about code reviews I am constantly reminded of the following

1. How hard it is to grok code that I've never seen before

2. How hard it is to grok my own code from 6 months ago

Sometimes it may take me a day or more to fully get my head around a new piece of code I am working with. I could not give a thorough AND fair review of another person's code without internalizing it to a satisfactory degree.

There is also the issue of software as art vs engineering. Quite often I will take issue with the style or approach of another person's coding simply because I would have done it differently had I done it myself. But that does not mean that their code is wrong.

All things considered, I think code reviews can be helpful in certain situations, but I think there are many pitfalls which must be avoided in order for them to justify the cost.

Re: We don’t have time for code reviews

#35
post #13

Proper code review provides much more benefit than cost and is indispensible for quality software. Improper code review is a waste of time (or worse). Proper code review is done: - by another programmer - by someone with some knowledge of the application - by someone with some knowledge of the environment - against some code standard - against standard requirements (APIs, database, etc.) - with a checklist - uniforml…

Hi, OP here. Interesting list. I think I would approve most of your points, but never worked (or created) such a "proper" environments in my various teams. Could you share the kind of checklist you use? Or give an idea of the kind of "checks" you have there?

Thanks!

Re: We don’t have time for code reviews

#36

You think that's bad, I once worked in a startup where the Product Manager told me he didn't have time for tests.

Its the whole point for me: even if I prefer managers that I can convince, I just call that "development". Replace: "feature is done but need to be tested or reviewed" by "feature is not done".

The fall in the technical/developer responsibility anyway for me.

Re: We don’t have time for code reviews

#37
post #15

Earlier quoted context omitted.

I think it's hilarious (and typical) that the moral of the story is "I don't push without code reviews" not "I don't use bad words in test code."

"I don't use bad words in test code." This is actually really important. We had this happen with a customer whilst UAT testing. The project schedules were under pressure and as a result the customer went nuts (rightly so) and we had to make some concessions to customer as amends. It isn't just curse / bad words. Equally as bad are comments like: "this was the customer's stupid idea not ours" Also, if you really have…

What about

  window.alert = function(){};

Re: We don’t have time for code reviews

#38

My co-workers write awful code, including the "architect". I try to review their code, without having a formal process, because I simply do not trust their abilities. It would not be pretty if we did code reviews.

Or it might be better. Once code reviews are happening people tend to start realizing that people are reading their code and it is raised to a higher standard.

There's also the added benefit that reviewing your code can teach them about better programming practices, and also that reviewing their code with them and asking questions about their thought patterns can make them better developers.

It's my opinion that if the code is awful then code reviews become especially helpful and important to bringing everyone up.

Re: We don’t have time for code reviews

#39
I care more about full functional test coverage than code review. The tests must be automated, regression and functional coverage not just unit test. Test results/logs must output in HTML store in DB backend (now) or excel (did that in 1997) for ease of analysis. Test failures are highlight in red, passes are in green.

The developers should write new test code for every new feature and run all the existing test code after/merge before checkin. All the projects I worked on that implement this process are very successful. With this process in place and agree upon, I careless about code review.

With enough "functional" test coverage, it is easy to do massive refactoring of code without worry about any breaking.

Worked on one project that try code review for two weeks - other than a few comments about coding style, not much gain from the time spent.

Re: We don’t have time for code reviews

#40
post #16
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.

My comments are more along the lines of alert("GOT HERE 1"); alert("GOT HERE 2"); alert("GOT HERE 3"); Not very helpful if someone else sees it, but also not profanity.

They can be quite irritating for others though, especially if they happen during a loop so that the user will have to click past 40 popups to access something.
Post reply on HN