5

What do you do when pull requests are enforced, but everyone is so fucking apathetic about reviewing code?

Comments
  • 3
    When enforced routines end up as bottle necks you either remove or circumvent them.

    Approve your own PRs.
  • 4
    Ask people directly. (Don't say "Can somebody approve my PR?" but rather "Hey $name, can you please check my PR?")

    Nag your teammates, but also respect their time. (If your PR is open for 15 minutes, chill, do something else. If it's not approved even after three days and multiple be nagging, escalate to your boss.)

    If process requires approval, waiting for approval is an artefact that happens.

    Do you do retrospectives? Because that's a classic topic for one. Had it a few times in my teams ^^
  • 3
    @devdiddydog Bad advise duck! The only PR bypassing the pr approval shall only be important hotfixes. (And those should still be reviewed afterwards.)
  • 1
    I agree, but the frustration in the OP gives me the impression it has been addressed multiple times already. I've been there myself. If you're trying to work agile and frequently push your changes out, but everything stops because of lack of code review - write more tests and start trusting them instead then.

    At some point you have got to do something. I'd rather push a single bug that might have been picked up in a review than to sit around for days waiting for my changes to go live.
  • 1
    hire a part time code reviewer or an external company?
  • 1
  • 2
    Tell your manager or lead. It's part of their job to make sure this isn't happening
  • 1
    I allways thought of it this way: if someone else has to review your code, you are not the only one at fault if something goes wrong. If they let shitty code from you through because they just glossed over the PR then they fucked up not you.
  • 1
    I too do the same
  • 1
    @devdiddydog I understand the frustration but unilaterally approving and merging your own PR without any feedback will increase the potential fallout in the team and may even put your company in harm's way.

    I know it sounds strange but it's not YOUR responsibility to deploy a feature. It's the responsibility of the entire teams.

    If somebody complains why your feature isn't live, say: "I am blocked by our review process." (Better, complain yourself that you are hindered by the process proactively.) But don't do these shortcuts.

    All they do is hide issues within the team, everybody rather sooner or later will do it the same way, and at one point people will wonder about the "sudden" spike of incidents.
  • 1
    @devdiddydog Also writing "more" test is not the solution. Test coverage should not change depending on an approval requirement.

    Furthermore, having your tests peer reviewed provides your teammates with the opportunity to understand your assumptions and to correct them.

    Your teammates are either too busy doing normal work that they can't properly approve. That means you all need dedicated time for reading reviews. (My last team was like: The hour after the daily standup is reserved for PR.)

    It may be that they don't consider it fun or beneficial which is a huge red flag as that indicates that they don't understand the reason and impact of doing proper reviews. ("What a waste of time.")

    Or it may even be that nobody in a team has a real clue what the other is doing and that they even are unable to do proper reviews. ("Looks good to me. Approved.")

    There is so much to be discovered why people are apathetic about doing pull reviews. That doesn't make the artifact a bad one.
  • 0
    You should have control over your own branch or fork. Not sure why you are so anxious to have your code get to master?

    If you are required to have your code there by a deadline, your company should definitely re-evaluate that, because you have no control over other people.
  • 0
    @k0pernikus I know and agree with all of this. But sometimes you just can't break through. So while I'd approve my own PRs I would simultaniously make a big point out of it until someone finally took action - if anything just out of fear.

    "Guys, guys, you have to set aside time for code review!"

    [No changes... ]

    "Guys.. you have to set aside time for code review!"

    [Still no changes..]

    "I'm just approving my own now, got no time to wait so it's cool."

    "......."

    [Changes happen]

    Maybe. :)
Add Comment