2

Very often I have to review pull requests of fellow programmers and I face the problem - what should I do when the code is not as good as I would expect? Should I reject the pull request or accept/merge even though I don't like the code? Of course, I reject when I see errors, but I can't keep rejecting again and again. This annoys my colleagues and they ask me to close my eyes and move on. But if I accept the changes I don't like - what code review is for? What can you recommend?

Bernie Noel
  • 137
  • 2
  • 3
    There's a difference between "simply factually incorrect", "this may be correct, but is so low-quality I cannot accept it into the codebase", and "I subjectively just don't like these parts". Which case(s) are you asking about? –  Dec 24 '14 at 12:15
  • It's impossible to say without knowing the exact specifics of the situation. The only way to maintain a certain level of code quality is to reach a consensus within the team on what's acceptable and what's not. It's just not possible for one person to keep all of this in check. If it really bothers you, you could offer to help them improve the code or do it yourself before merging. Maybe think about introducing pair programming practices so more experienced developers can pitch in earlier and influence the design more. – toniedzwiedz Dec 24 '14 at 12:15
  • I'm talking the second one, when the quality is low that I wouldn't want to see it in our code base. But my colleague tells me that it's OK and the code needs to be merged ASAP, so I can stick my quality concerns... etc – Bernie Noel Dec 24 '14 at 12:16
  • 2
    You and your colleague disagree on what constitutes 'low quality'. It's not unusual to have differences of opinion like that. If you can't resolve it yourself then engage the rest of your team, or others from outside the team. If the code genuinely is low quality then it should not be hard to find opinions to reinforce your own. – Andy Brown Dec 24 '14 at 13:55
  • @AndyBrown - not necessarily. To me it sounds that colleague agrees that it is low quality, he just thinks that deadlines come first. – Davor Ždralo Jul 08 '15 at 15:41

0 Answers0