327
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-...
- 5,280
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 ...
- 45.9k
249
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 ...
- 2,837
229
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 ...
- 24.2k
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 ...
- 23.2k
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 ...
- 73.2k
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 ...
- 5,925
120
votes
Is it effective to review code in language I don't know?
Any sense? Yes. Even if you know nothing about the semantics of a programming language, you can still read characters and notice inconsistent formatting, missing comments, badly chosen identifiers, ...
- 108k
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 ...
- 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 ...
- 75.9k
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, ...
- 72.4k
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.
- 33.6k
82
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 ...
- 11.3k
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 ...
- 5,438
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 &...
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 ...
- 4,684
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 ...
- 47.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 ...
- 1,499
56
votes
Is it effective to review code in language I don't know?
As a regular contributor over at Code Review Stack Exchange, I encounter a lot of questions suffering from Language-agnostic issues, for example:
Formatting, indentation
Scope
Loops
Type operations
...
- 669
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 ...
- 147k
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, ...
- 1,914
47
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 ...
- 147k
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 ...
- 33.6k
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 ...
- 80.4k
44
votes
Is it effective to review code in language I don't know?
General advice
Here's the bottom line, in my opinion:
If you don't know the language well enough to know the features and common idioms, you are probably not going to contribute much to the review. ...
- 5,399
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 ...
- 2,211
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 ...
- 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 ...
- 57.8k
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 (...
- 7,699
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 ...
- 199k
Only top scored, non community-wiki answers of a minimum length are eligible