How to do efficient code review during release fever?

Question

The deadline for a release is tomorrow, your collegue finally finished his task that's crucial for this release, project manager is standing over your shoulder and presses you to finally make a build and you notice a flaw in your collegue's code during review. Not a critical one, but something you wouldn't let go if it weren't for the release tomorrow. And to make things worse, you have your own work you need to finish ASAP. So, what do you do? Do you raise your objection despite the pressure or do you just let this one slip?

One way I found is to temporarily merge this commit on a different branch and leave review for later. It works if the issue is just a cosmetic one and if it's the only one still waiting for code review. However, is there a more efficient way to handle this? For example, would you recommend commiting one person to only code review and tests?

Answer 1

The answer here is to communicate.

1. Tell the technical/team lead about the issue
2. Talk to QA about the potential impact
3. Tell project management (who is right behind you) that there might be an issue that causes the release to be delayed, and you will get back with them ASAP (in a matter of minutes or hours)
4. Evaluate whether this issue is a show stopper for the release

If the issue is not a show stopper, continue with the release. Inform QA, and your users of the issue and commit to a follow-up date where the problem will be patched. This just becomes another risk going to production.

If this is a show stopper (meaning it will have a negative impact on the bottom line or someone's health) communicate this up the chain that your recommendation is to delay the release, and tell why. Then get back with the dev and work out how long it will take to fix the issue, and tell management that you need X number of minutes/hours/days.

Chances are management will not be happy about a show stopper this late in the game, but will not want that released to production.

Communicate, and let management make the call.

Answer 2

Don’t just communicate the problem, document it

My big concern with the other answers so far: Anything you say along these lines to the typical project manager facing an imminent deadline is likely to be ignored or forgotten. Then, you can still end up being on the hook for insufficiently communicating the risk, if something goes wrong.

Let the project manager know about the issue you have found, and let him know you will document it. You need to be able to point to your due diligence.

Where to document and whom to tell depends on your work environment, but definitely include your boss.

Identify Risk and Impact

You mention the problem is not a critical one but don’t really define what that means. Fleshing that out is your next step.

Do a quick risk and impact analysis identifying the problem, how likely it is to cause a problem (risk) and the severity of the consequences if the risk comes to fruition (impact). Use well-defined terms (that your project manager ought to know) like those found in the link above, but also provide a description backing up your analysis.

Your documentation should also include your recommended course of action. Yes, it is OK to raise a concern and still recommend proceeding with the release. It’s correct to identify the risk.

When is your next release?

If, after completing your risk/impact analysis you are still on the fence about what to recommend, take your release schedule into account. Some imperfect code is OK to release if you can expect to include the fix in two weeks.

If there is the chance that fixing your concern will get “deprioritized” (that is, neglected in favor of the next shiny enhancement) then that is one more reason to document the issue as soon as possible after you discover it: if effectively “starts the clock” on the issue.

Answer 3

In this case, just inform everyone and let them decide. It's probably not the first bug you're released.

The deadline is tomorrow, but you find yourself in this situation:

project manager is standing over your shoulder and presses you to finally make a build

This may be the exception, so don't make any drastic changes to your process just because one bug slipped through. What you should do is put some measures in place, so you're not making builds at the last minute. Hopefully, you're making builds at a regular enough frequency to give you confidence it will succeed. That's still not a good enough reason to run them last minute. Having things well tested is another piece to increase confidence.

Present this scenario to whoever is leading this thing.

• Determine what types of problems should be presented.
• Identify the options.
• Who makes the decision
• When should all of this be decided by? It's probably going to be relative to release date.

Although this doesn't answer what to do with this last minute problem, it does offer a way to make sure you're able to deal with them in the future. Especially when there is pressure to release, you want to have a way to handle things in a way that is thought-out and not driven by emotions.

Answer 4

If there is a deadline, and your manager is right behind you looking over your shoulder, you tell him to go away. He goes away or you go away. Explain to him that being forced to review under pressure, you might as well not review the code.

In a situation like this, you can be quite sure that some critical bug will go through.

You may be in a lucky situation, like submitting to Apple's App Store, where you have a few days to retract a new version. But if your code gets shipped to customers, that's a recipe for disaster. Next time I hope your manager plans better.

Answer 5

Other answers focus on the problem of your manager trying to bend your quality process.

However, what I think you are asking is how to deal with the fact that code review requires at least 2 people, and therefore introduces significant delays. For example, if a task takes 2h to implement and 2h to review, often there's a long pause between the two activities. In real time the task might take 2 or 3 days to complete.

This is a classic problem of throughput vs latency. In software producing machines(human beings) context switches are expensive, so preempting someone's job to immediatelly do a review shouldn't be a common practice.

Here are some tips to mitigate the problem:

• While working on a task, send code in small parts, so that the reviewer won't have to reserve long continous chunks of time.
• Promote a culture of high priority for code reviews. Don't stop your current unit of work when you receive a request, but when you are wondering about what to do next, always choose a review from your queue.
• Take advantage of timezone difference in your teams, if there's one.
• Do not commit a single person to only code review. It's contrproductive. He will not be able to handle the load, if he's going to be serious about his task. Your manager will not accept the idea of someone being idle, just to potentially save one day.