Settings

Theme

Ask HN: How to deal with coworker code review who haven't read my code?

3 points by tomazio 4 years ago · 10 comments · 1 min read


I started a new job half a year ago as a senior full stack engineer. There is one coworker who routinely reviews my PRs and leaves feedback that clearly demonstrates they didn't read my code closely or they didn't fully grok it. I then have to go out of way my to prove to this person the code is doing what is expected/asked, either in long winded responses on the PR, or sometimes as far as pulling them into a video call to screen share and live debug the code to them. It's becoming increasingly annoying, and even a little exhausting which is hurting my motivation and delivery throughput. Anyone ever had similar experiences or have any tips to deal with this type of situation?

MaknMoreGtnLess 4 years ago

> I then have to go out of way my to prove to this person the code is doing what is expected/asked, either in long winded responses on the PR, or sometimes as far as pulling them into a video call to screen share and live debug the code to them

Process issue. Fix:

1. Write tests

2. Ensure tests can run in a well defined/containerized environment so that they can run tests and ensure they pass

draaglom 4 years ago

I don't have answers but some questions that might help:

* Are their code reviews _of you_ poor in particular, or are they poor in general when reviewing anyone's code?

* Are they reviewing your code in good faith (and are just finding it challenging) or in bad faith (and can't be arsed or don't like you / the project / etc)?

* Have you had a direct conversation with them about it? (Is your relationship with them strong enough to enable that?)

* Do your team have standards for a code review? Expectations for an author and for a reviewer?

  • tomazioOP 4 years ago

    1. It seems to mostly be a me problem, on reviews of other coworkers PRs her comment seem reasonable. That could just be not fully understanding scope or task at hand. 2. I believe it's in good faith. Sometimes comments will be helpful for when I forgot a specific convention or missed something small. 3. I have not had a direct conversation as I'm not sure how to go about it or phrase it. We seem to have a good relationship and I don't want to potentially ruin it going about this in the wrong way. 4. We have basic standards: X amount of required reviewers, all PR feedback resolved appropriately, PR contents fully address acceptance criteria for the task at hand. Pretty standard stuff in my experience for code review

    • draaglom 4 years ago

      >It seems to mostly be a me problem, on reviews of other coworkers PRs her comment seem reasonable. That could just be not fully understanding scope or task at hand.

      What do you think explains the difference?

      * Are there ways you can explain yourself better that others are doing?

      * Could the problem honestly be with your code being hard to understand? Could you simplify?

      * Could you deliver in smaller chunks, that are each easier to understand?

      * Is it about working on a tech or product area that they aren't so familiar with? Perhaps you can help bridge that gap with a lightning talk or some new documentation?

      >I have not had a direct conversation as I'm not sure how to go about it or phrase it. We seem to have a good relationship and I don't want to potentially ruin it going about this in the wrong way.

      You've got to place trust in your colleagues that you can have adult conversations about what's working well and what's not. It doesn't have to be a big deal!

      It could be as simple as:

      "Hey <name>, I feel like a few times lately PR reviews hasn't been smooth. Have you noticed that too? Is there anything you think I could do differently?"

      >We have basic standards

      One thing that could help here is improving your team standards for a PR author! E.g. at my company, we have a PR template which encourages you to add a screenshot/screencast of your feature working.

      We also encourage adding test steps to our PR template, to make it easy for reviewers to try the thing themselves. Good test steps might look like:

      * Run the app as normal

      * Turn on feature flag XYZ

      * Head to localhost:3000/new-page and click all the widgets

      * You should see the widgets created in your db with this query:

      It sounds like work, but it's probably the steps you did anyway before you put the PR up! so it's more of a mindset thing

bjourne 4 years ago

Your code may be messy or written in a non-obvious way. Does your coworker occasionally suggest how your code could be improved? Do you take those suggestions to heart?

ipaddr 4 years ago

I hate that. I prefer an environment that enforces individual responsibility. I'll push through your code without much review so you need to own your code and make sure everything works.

AnimalMuppet 4 years ago

I would at least consider talking to the co-worker's boss.

Barring that... is there any way you can route your PR reviews to someone else?

  • PragmaticPulp 4 years ago

    > I would at least consider talking to the co-worker's boss.

    No, the OP should talk to their own boss.

    Escalate to your own manager. Let the managers manage it.

    • AnimalMuppet 4 years ago

      Yeah, you're right. Your approach is much better. Like, really, don't listen to my post, listen to PragmaticPulp.

      No, not sarcasm. I don't know what I was thinking, but I was clearly giving bad advice.

    • bradwood 4 years ago

      Talk to the _person_ first. If that yields no joy, then escalate to your own manager

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection