gets worse when you hear "we don't have time for unit tests"
We don’t have time for code reviews
21–30 of 67 posts
Re: We don’t have time for code reviews
#22linkbait. Title is no time for code reviews and talks about how they do code reviews.
Re: We don’t have time for code reviews
#23Earlier quoted context omitted.
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.
That's nice in theory, but in practice another dev really should be running the code and doing some manual testing. Having full knowledge of the innards allows a competent dev to foresee potential trouble spots in the architecture and hit some edge cases to make sure they don't break.
CI may run the tests, but that assumes your tests have captured every possible edge case. It may be the intention of tests to be comprehensive and cover everything under the sun, but that is rarely the case in reality.
Ditto QA - they should be able to black-box test the software and all of its possible states, but in reality something is going to pass through the net.
Having a dev run the code themselves and poke around in it is just a plain good idea.
Re: We don’t have time for code reviews
#24Earlier quoted context omitted.
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.
That's nice in theory, but in practice another dev really should be running the code and doing some manual testing. Having full knowledge of the innards allows a competent dev to foresee potential trouble spots in the architecture and hit some edge cases to make sure they don't break. CI may run the tests, but that assumes your tests have captured every possible edge case. It may be the intention of tests to be compr…
It might be a somewhat good idea (in my experience, the scenario you outline is significantly less likely than domain knowledge by QA hitting patterns the developer with limited domain knowledge had not anticipated), but it's not a necessity for code review to be a good idea.
Re: We don’t have time for code reviews
#25Earlier quoted context omitted.
Are code reviews by non-programmers actually common? How can that possibly work?
I've seen 'code reviews' that were in fact functionality checks. So it's more like a 'check if this code works'. And I can tell you: a web form capable of SQL-injections is still 'working code'. So it gets a positive 'code review'.
Sounds like there's no code being reviewed, how can it be called "code review"?
Re: We don’t have time for code reviews
#26Proper 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…
I also include junior developers who don't know much about the application or environment in code reviews. It's a good way for them to learn the code base and see what type of defects the senior developers point out.
Re: We don’t have time for code reviews
#27Agree 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.
Using pull requests as your code review doesn't force you to not run the code. My team uses forks of upstream repos and all merges upstream happen via a PR that is peer reviewed. It isn't uncommon for us to pull down the changes of a PR locally to "play around" with a new feature or change to existing functionality.
We have tests in place to help stop regressions, but sometimes seeing a change in action can really help to put the corresponding code in context.
Most of my team uses hub, but you can check out a pull request easily enough with git:
git fetch +refs/pull//head
git checkout FETCH_HEADRe: We don’t have time for code reviews
#28Pull 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.
Re: We don’t have time for code reviews
#29My 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.
Re: We don’t have time for code reviews
#30Proper 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…
Are code reviews by non-programmers actually common? How can that possibly work?
Bots that scan code for syntax/quality. While these are useful they should not qualify as a complete code review.