Skip to main content
328 votes

How do I handle disagreement in a code review regarding an unlikely edge case?

I can give you an example of a corner case that could never occur that caused a disaster. When the Ariane 4 was being developed the values from the lateral accelerometers were scaled to fit into a 16-...
Steve Barnes's user avatar
  • 5,320
312 votes

What do you do when code review is just too hard?

The premise of the question is, frankly, astounding. We suppose that there is a large change to fragile, complex code, and that there is simply not enough time to review it properly. This is the very ...
Eric Lippert's user avatar
  • 46.3k
248 votes

What to do when code submitted for code review appears to be too complicated?

If it cannot be reviewed, it cannot pass review. You have to understand that code review isn't for finding bugs. That's what QA is for. Code review is to ensure that future maintenance of the code is ...
Kevin Fee's user avatar
  • 2,837
230 votes
Accepted

How do I handle disagreement in a code review regarding an unlikely edge case?

an obscure case that is extremely unlikely to happen--in fact I'm not sure it is even possible to occur Not having untested behaviors in code can be very important. If a piece of code is run e.g. 50 ...
9000's user avatar
  • 24.3k
181 votes

How can I defend reducing the strength of code reviews?

How can I justify and defend the thesis that: The merge button should be enabled by default The code review should be a recommendation , but not mandatory The code author should have the right to ...
Philip Kendall's user avatar
141 votes
Accepted

In code reviews, should the reviewer always present a solution for issues?

As a reviewer, your job is to check if a piece of code (or a document) meets certain objectives that have been agreed upon before the review. Some of these objectives will typically involve a ...
Bart van Ingen Schenau's user avatar
129 votes
Accepted

How to find positive things in a code review?

How to find positive things in a code review? After some serious quality problems in the last year, my company has recently introduced code reviews. Great, you have a real opportunity to create value ...
Aaron Hall's user avatar
  • 5,953
107 votes

How to find positive things in a code review?

Don't bother picking something good unless its a solid concise example and is directly related to the focused issue. I won't sugar coat it - from the sounds of it you are dealing with at least one ...
plast1k's user avatar
  • 1,559
100 votes

What do you do when code review is just too hard?

One of the primary goal of a code review is to increase quality and deliver robust code. Robust, because 4 eyes usually spot more problems than 2. And the reviewer who has not written the additional ...
Christophe's user avatar
  • 78.7k
97 votes

How to find positive things in a code review?

Code reviews can be toxic, time wasting, will to live-sapping nerd wars. Just look at the divergence of opinion on things like clean code vs comments, naming conventions, unit and integration testing, ...
Ewan's user avatar
  • 77.5k
87 votes

How do I handle disagreement in a code review regarding an unlikely edge case?

Since it wasn't handled before, it's out of scope for your effort. You or your colleague can ask your manager if it's worth the effort to cover this case.
kevin cline's user avatar
  • 33.7k
83 votes
Accepted

In code review, should I ask to do a refactor outside of the scope in a pull request?

There are several relevant trade-offs here: Review complexity. If a branch has more than one functional change commit or more than one refactoring commit it becomes time-consuming to review the ...
l0b0's user avatar
  • 11.5k
73 votes
Accepted

Having a flag to indicate if we should throw errors

The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the ...
mmathis's user avatar
  • 5,498
73 votes
Accepted

What is the best way to code review a work-in-progress?

GitHub allows for PR to be in a "draft" state. Your team can see the differences, and even comment on it, but it's still obviously a work-in-progress, and cannot be merged until you click a &...
FrustratedWithFormsDesigner's user avatar
73 votes
Accepted

Code review from domain non expert

Code reviews are not only useful to find potential errors, but also to transfer knowledge between developers. It is totally appropriate to ask domain specific questions, ask for reasons why problems ...
Rik D's user avatar
  • 4,800
67 votes
Accepted

Agile Practices: Code Review - Fail the review or raise an issue?

are there any inherent problems or considerations with raising a ticket off of the back of a review, instead of failing it? Not inherently. For example, the implementation of the current change may ...
Flater's user avatar
  • 52.7k
57 votes

How do I document my code for minimum time review?

IMO the best documentation is the documentation you don't actually need. I also hate writing documentation and comments. With that being said: Pick readable and talking names. Don't use n, but ...
Mario's user avatar
  • 1,509
53 votes

How do I handle disagreement in a code review regarding an unlikely edge case?

With complex algorithms, it's very difficult to prove you have thought of every test case that will come up in the real world. When you intentionally leave a test case broken because it won't come up ...
Karl Bielefeldt's user avatar
52 votes

Ways to explain code when told it doesn't make sense

Personally, I have encountered multiple variants of code-that-is-hard-to-understand, and each needs a different way to cope with: Simply messy. Formatting is off, variable names are not clear, ...
mtj's user avatar
  • 2,058
48 votes
Accepted

Code review workflow where pull request author must merge

In the first case, it's usually a courtesy. In most organizations, merges kick off a series of automated tests which must be dealt with promptly if they fail. Especially if there was a significant ...
Karl Bielefeldt's user avatar
47 votes

Having a flag to indicate if we should throw errors

Is this a good way of handling exceptions? No, I think this is pretty bad practice.  Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's ...
Erik Eidt's user avatar
  • 34.2k
45 votes

What to do when code submitted for code review appears to be too complicated?

Everything that you mention is perfectly valid to point out in a code review. When I receive a code review, I do review the tests. If the tests don't provide sufficient coverage, that's something to ...
Thomas Owens's user avatar
  • 83.4k
43 votes

How to give feedback on a badly reviewed PR

Advice for gracefully giving feedback would fall into the "career advice" category of questions, which is off topic for this community, however there are some things you can do from a ...
Greg Burghardt's user avatar
40 votes

What do you do when code review is just too hard?

Welcome to the world of legacy software development. You have 100s of thousands, millions, 10s of millions of lines of code. These lines of code are valuable, in that they produce a revenue stream ...
Yakk's user avatar
  • 2,199
38 votes

How to gradually introduce code reviews?

The first step would be to walk up to someone and say "hey, would you review my code?". Be the change you want to see in your organization. If you can't find a single individual willing to do it, you ...
Bryan Oakley's user avatar
  • 25.3k
38 votes

How do I handle disagreement in a code review regarding an unlikely edge case?

This is not a technical question but a business strategy decision. You notice the suggested fix is for a very obscure case which will almost never happen. Here it makes a big difference if you are ...
JacquesB's user avatar
  • 59.9k
38 votes

Agile Practices: Code Review - Fail the review or raise an issue?

At the end of a 2 week sprint and a task has a code review [...] Easy refactor job. Why does that pop up at the end of the sprint? A code review should happen as soon as you think the code is done (...
nvoigt's user avatar
  • 8,202
38 votes

How can I defend reducing the strength of code reviews?

How do the company's stakeholders feel about the productivity of their developers? My previous boss told me a story once about this kind of zealotry. The developer team was using that zealotry as an ...
Robert Harvey's user avatar
38 votes

Code review from domain non expert

When I was a kid (21 or so) I looked over some programmer’s shoulder, pointed at the screen, and said “there’s a bug”. He protested loudly, I had no idea what the code was doing etc. My mate heard it, ...
gnasher729's user avatar
37 votes

Having a flag to indicate if we should throw errors

They have worked on critical applications dealing with aviation where the system could not go down. As a result ... That is an interesting introduction, which gives me the impression the motivation ...
Doc Brown's user avatar
  • 210k

Only top scored, non community-wiki answers of a minimum length are eligible