How to discuss over code review?

Question

Recently we have finished a project using React 16 with typescript. We have developed a good readable, maintainable , moduralized and following all coding standards with advance methods as much as possible and QA has passed it with zero defects.Now when it sent to code review team (which we never meet) and it was rejected with single statement that it has built with very poor logic.

I have been working in IT industry since 10 years and always followings tha latest technology and applying the logic which fits to the problem along with style guide which has been recommended, for instance we have user React hooks, functional components over class components, es6 methods to keep immutability in mind and functional programing paradigm and proper documentation.

Now we have meeting tomorrow, I have my own points but my main concern is my aggression.

So my question is how to handle such situation when reviewer have little less knowledge about the technology. As he has been appointed as a reviewer because he is senior and core team which gives approval.

I strongly agree that there are always scope fo improvmnet in code but saying it poor logic hurts a lot to whole team and now it's my responsibilities to fight back with reviewer.

Kindly help me to prepare for this situation and win over this.

Answer 1

Kindly help me to prepare for this situation

I offer the standard advice regarding code reviews.

• You are not your code.
• Learn not to be offended or aggressive during code reviews.
• Make sure to focus on the code, not the people.
• Don't be so defensive.
• Remember that, even though you have been working for 10 years and know all the latest technology, the code could still have problems.
• Go into the meeting relaxed, prepared, and open to suggestions.
• Assume positive intent.
• Use it as a learning experience (thanks @EdHeal).

Answer 2

Ask for examples of better logic, and a discussion about why it is better.

If your logic actually is poor, it is probably because you don't fully understand the problem to be solved. And if you actually have ten years experience and you don't fully understand the problem to be solved, it is probably because the specification is incomplete or wrong. This means that your logic may be the perfectly sound and correct solution for the wrong problem.

If this is true then it will become quite apparent at the meeting because the senior reviewer's examples of better logic will not work with your model of the problem. Then you can figure out which of you has the wrong problem in mind.

Answer 3

Recently we have finished a project using React 16 with typescript. We have developed a good readable, maintainable , moduralized and following all coding standards with advance methods as much as possible and QA has passed it with zero defects.Now when it sent to code review team (which we never meet) and it was rejected with single statement that it has built with very poor logic.

How does code review come after completing the project or even after QA? So you have a completed project and have now been told to do it from scratch AFTER it is ready to ship?

On my team, code review follows any commit to the working branch so that changes can be done in dev and tested. Changing code now on your team means redoing QA and redoing the project completion work.

You have a terrible manager somewhere. Act accordingly.

Answer 4

So you (plural you) have finished a project, and this is the point where it is reviewed? Well, whoever planned it has made a giant blunder. Reviews should happen whenever you have done about a day's worth of work, not when your whole team finished a project.

Let's look at it from the company's point of view: They have the choice of accepting it as it is, or accepting it with a small amount of necessary fixes, or follow the reviewer who says the logic is unacceptable and spend months on ripping it all out and rewriting it. Guess what the company will go for. Will they delay the project for months and pay your team for months to fix things? I DON'T THINK SO. You mentioned "tight deadlines". The tight deadlines will obviously not be met if management listens to the reviewer.

Now if your project doesn't work (doesn't pass QA), that's something else, but if it works, this senior reviewer will have some hard time convincing management that months of extra work is needed.

That said, as a senior developer I have my favourite ways of doing things, and I would like you to do things the way I prefer them, but I'm adult enough to know that there are other ways that are fine as well. Maybe second best, or third best, but the third best way to do something is usually good enough. And I wouldn't tell you to change things because they are not done the way I prefer, but only if things are objectively bad.

Go to the meeting with an open mind. Stay calm all the time. Maybe some camomile tea before the meeting starts, but not too much. There's a rule: The one who shouts is wrong, so don't shout. If the reviewer turns out to be right, admit that he's right. If he is not right, state what you think calmly. You can say that you don't agree that your logic is wrong, or that you absolutely don't agree that your logic is wrong. Calmly, without shouting, that's important. If you shout then people think you disagree because you can't look at your own work objectively and can't stand being proven wrong. If you say it calmly without shouting, they'll think you disagree because you are right. You can also say you don't agree his approach is better, or that it is definitely not enough better to justify lots of work.

At some point some decision has to be made. If it's up to the reviewer to decide, then you write down what that decision will cost the company, that you don't agree with the decision and why, and how much it will cost if you don't go with that decision, and you give that to someone whose budget is affected by a lengthy delay.

You need to do that before work starts. So if complaints come in after three months about the delay (when the manager whose responsibility it all is gets pressure about the delay), you can point out that your team told the manager that all this wasn't needed, and you are not stuck with the blame.

Answer 5

Receiving a terse critique can be quite off-putting and alarming. The quality of the code and quality of the communications are issues that should be addressed separately.

I suggest to first, be professional and patient in the review discussion - avoid reacting defensively to the criticisms of the reviewer, rather thank him/her for the input and resolve to make changes that will enable the project to pass review.

Second, after the review is complete, you should consider offering feedback to the reviewer about his/her communication. It is important that the reviewer hears how his/her choice of words and medium impacted you. Some keys to offering productive and professional feedback are:

1. Always provide feedback in person, or over the phone - never over email, text, or indirectly through another person.
2. Be direct about your intentions in the conversation - don't veil feedback or initiate a conversation with false pretenses.
3. Share your factual observations - e.g., "Your review included a brief note only saying _______."
4. Share how the actions you observed impacted you personally - e.g. "I felt like ____."
5. Invite a reflection - e.g., "Thank you for letting me share that, what do you think of the that observation?"
6. Resolve to do better, together. e.g., "In the future, could you call me to discuss your critique? I'd like the opportunity to learn through questions and discussion."

I hope both the review discussion and your follow-up with the reviewer are productive and helpful discussions for you.