I know we shouldn't feel defensive.
But we do, don't we?
For further actions, you may consider blocking this person and/or reporting abuse
I know we shouldn't feel defensive.
But we do, don't we?
For further actions, you may consider blocking this person and/or reporting abuse
Liam Hall -
Jagroop Singh -
Bryan Primus Lumbantobing -
Waradu -
Top comments (26)
As everyone is now doing test-driven pair-programming all of the time, why would we even need code reviews anymore?
</irony>
But seriously, there are many possible reasons to feel defensive or uncomfortable in a code review. Maybe you had to finish something in a hurry and hope to get it shipped before an important "deadline". Maybe you had to rewrite "your" code several times to meet peculiar formal code style requirements that your coworker isn't aware of. Maybe you're in a probabation period and anxious to make a good impression. I've had all of this, including "public" code reviews in a meeting room with a bunch of coworkers delegated to stare at our PR "to learn something" , but the most annoying code review situation was when I refactored some nice code to meet the change requests of the first review, only to defend those changes against the second code reviewer, reviewing my refinements and requesting to revert the changes back to my original draft.
I did at first; I don't anymore.
Don't take it personally. You are not your code. If someone suggests a different way to do something, it is an opportunity to discuss. Maybe your way is better; maybe there's another way to approach the problem.
Keep in mind that if you are having your code PR'ed, it is being written for the team. The best communicators are the ones who can adapt what they're saying so that everybody regardless of skill level understands; the same is true with code.
You will not be the owner/sole maintainer of that code forever, even if you want to be (unless it's truly your own project, in which case you have the right to ignore the PR). Try to reach a compromise for everyone to agree.
Depends on the reviewer's style of communication. If it's clear that the reviewer's intention is to help me mold a bulletproof piece of awesomeness then their review SHOULD always be as detailed as possible and should not hesitate of using the hardest criticism as possible.
However if it sounds as their intentions is simply to disregard my implementation, because it doesn't fit their taste then I may get defensive.
In the end it's all written and missing nuances of the spoken word, so both sides need to be careful with interpretations.
I agree.
When I have the feeling that written discussion is leading us nowhere very slowly, I jump to a slack call and share my screen. It usually helps a lot.
I would say it depends on how much back and forth happens about what we did vs what the reviewer was expecting and how that is communicated.
This can be hard sometimes but depending on the company culture/standards, it's better to try to let go any personal attachments and just address the reviews and be done with all that since at the end of the day the code won't belong to us.
For work, I'm my own code reviewer (it's a small company).
As for contributing to OSS, I don't usually get defensive, no. I go in with the assumption that any PR I make might not get merged due to factors outside my control, but I'll argue the case for why it should get merged, unless something comes up during the discussion that changes my mind.
For nitpicks about code style etc I usually implement the requested changes without question.
There is one OSS project that's an exception, though, which has a weirdly insular and toxic community, especially one or two contributors who seem to take great delight in raising pointless objections to any attempt to approve the low quality of its code base... I won't name and shame, but such projects definitely exist. A lot depends on the community around specific projects.
Because it feels like your baby, the thing you have created is now being altered.
But to say it better, code changes all the time, next week the intern might have removed the code because the function specification was changed or whatever so just don't get to attached to your own code.
It's our baby and we are encouraged to code with a perfectionnist mindset, have strong opinions on how to do it right, produce clean code, respect all good practices, remove that newline would offend our sense of beauty, ...
And then we submit a pull request and suddenly we need to be like an ego-less buddhist monk that see things with a beginner mind and welcome challenges from everyone.
It's a tough dance to master :P
Sometimes??? Most PR reviews degenerate into arguments (usually only within a team, not normally on open source stuff). Programming is a deeply personal thing, it's very difficult not to take review comments as a slight against you.
Two things to keep in mind when you're feeling defensive:
I like your second point, there are indeed nature PR of different natures and if we recognize that, we can explicitly invite people to challenge us hard for PR that will have lots of repercussion down the line, but not do it for all PRs which would be wasteful
The 1st one I ever had yes, I was a bit defensive but I decided to not engage till the next day after my team destroyed the PR and I cooled off. I feel like they had been waiting for the opportunity since forever. After that they no longer destroyed my PRs and were pretty cool about it.
But it's been a while since I've been on a team that actively reviews code this way lol
It's hard not to be, you've spent hours making a solution to the task you are given and think its ready and doing what it should, then you get critiqued on some point which then makes you feel you didn't do a good job. I've given constructive feedback which has at times riled someone's feathers because he thought he had solved just the problem he was given, but missed how the code was actually used. Took a lot of diplomacy to get past that.
When we still threw code over the wall to QA it used to be said in one place, "Developers code and give to QA, whose job is to tell Developers where they missed the mark."
That's so true, QA used to be the ones easing that tension!
Now we developers are both judge and defendant :)