0

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.

  • 1
    Reviewer did not comment any specific code and just mailed that code is based on very poor logic which hurts the most – diEcho Jan 19 at 12:30
  • I agree with ur points, there is always learning in programing world, I m happily open for that but still it hurts – diEcho Jan 19 at 12:35
  • What's your relation with a reviewer? Are you employed at the same company? Do you talk every once in a while or do you only communicate via mail? – Bernhard Döbler Jan 19 at 12:42
  • We have only communicated via email and for the first time and yes he is in same company but what i got feedback from other team about him is that he is arrogant kind of person and less skilled. – diEcho Jan 19 at 12:45
  • Will you please proofread your question and clean it up? There are a lot of typos, and it makes it difficult to read. – Kat Jan 20 at 0:41
16

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).
  • But my code is my indentity in workplace. Isn't it true? – diEcho Jan 19 at 13:04
  • 1
    and use it as a learning experience – Ed Heal Jan 19 at 13:22
  • 6
    "Assume positive intent" is a great point on its own and the most important to remember imho - great answer Joe. – Jay Jan 19 at 14:54
  • 3
    Something that helps me going into code reviews is the mindset of "It's the team's code." Code reviews are a way to take code one individual wrote and give the team ownership over it. – Tyzoid Jan 19 at 22:01
  • 1
    @JoeStrazzere all good points, but IMHO they don't apply to half-assed code reviews. CR should really be more informative and proactive, a single-statement CR is rubbish and has no value at all and is a bad CR. Where's the positive intent in this? – Czar Jan 20 at 9:12
13

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.

6

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.

  • Yes. True, Manager and Management is terrible. We have raised PR since I month ago and now they are reviewing and gives comments in mail.but not over code. Which is also annoying too. – diEcho Jan 20 at 6:33
  • 2
    @diEcho work furiously on that resume. – Matthew Gaiser Jan 20 at 7:12
0

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.

  • Thank you for valuable suggestions. Yes Project is being passed with zero defect by QA and if it is being delayed then project budget will be impacted ( as I was told by my line manager) . Problem here is my Line.manager is non technical and he belive what the CR team said without any other thoughts. I have to fight for our team and for myself and will keep things calmly. Thanks agaib – diEcho Jan 19 at 14:49
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.