There is a balance to be found between pointing out what is going to cause problems and what you find personally inelegant. What I find personally inelegant I t

There is a balance to be found between pointing out what is going to cause problems and what you find personally inelegant. What I find personally inelegant I typically leave as a suggestion, while approving the MR as a whole. The contributor - who has spent more time than me thinking about his code in the context of the issue - can finish things up on their own, taking my feedback into account where they see fit. This works well for most people. Very junior developers or those new to the team/tooling might need more explicit guidance on more mundane matters, so I will be more stringent on those matters, at least for the first few tickets. If I have a serious concern I will also message/talk to the person directly. We need to reach an agreement going forwards, rather than trying to "play the game" and slip code past each other's standards. Direct communication creates good relationships and honest intentions between developers. MRs can be a good opportunity to publically ariculate design decisions and trade offs. In that sense, they are good for learning. Small, well-defined, succinct issues/MRs can be an invaluable form of documentation, helping you to reconcile bugs with expected behaviour months or even years down the line. I think it's important to keep MRs fast. The more friction there is to merging work, the less likely developers are to break up and integrate their work in clean, intelligible, atomic commits.