I am a senior software engineer in a company who has the role of code reviewing.
Last week I have rejected two PRs from juniors, one of them was simply duplicating a lot of code (~5K) lines with the names of the functions changed.
The junior engineer urged me and other reviewer to accept the PR and I quote
I want merge it and close the PR and the ticket as it has been open for a while, then we can make your requested changes later., and was very upset as me and the other reviewer rejected the code to be merged as we see it will cause huge problems in the future.
The other PR was not that bad, but it's introducing rigid inflexible APIs that will break compatibility in the future. ( FYI, existing APIs are very inflexible and we want to change them to more flexible generic solution ). We have had 2 design meetings before about these APIs in 6 months time-frame. I quote the following
Me: Adding these non-generic inflexible APIs adds very small value and may not serve the requests from other teams. We will also remove it later.
JE: Adding these APIs is not a problem, as we will remove them later and provide generic APIs. We already have inflexible APIs
Me: But this will make the effort on the users high, as it breaks compatibility
JE: We will break compatibility in all cases, so we can add it and remove it later. This is highly requested feature and had been requested a year ago.
Me: Okay, whatever you see.
We were very considerate in our words that we only talked about the code itself and not the engineers in any way.
Our scrum master asks us that we shall coach the juniors behaviorally and I disagree.
Is it my role as senior engineer to teach people that this is normal in the software industry ? To teach them how to act behaviorally in design/review meeting.
PS: My company goes for no-blaming culture at all, and I feel that it's more like no-accounting culture instead of no-blaming.