17

It's been a couple of weeks since I started my new job as a mid level web dev.

There's an ongoing project on which only me and the senior developer (who is also new here) are working on. As in most teams, changes come in pull requests and someone else has to review your code to merge it.

Once I got the hang of the codebase I started creating a lot of PRs that contained features/patches/fixes mentioned in our ticketing system, but the last couple of days I see my PRs stacking up and not getting merged.

Initially, I though that my coworker didn't have enough time to review. Unfortunately, I found out that he is not merging my code intentionally even though this slows the whole project down.

I fear he is feeling threatened by my performance and he is trying to artificially slow me down. How am I supposed to deal with it?

Let me answer some questions before they come up:

  • Q: Are your PRs too large to review and he feels fatigued?

    A: Most PRs include no more than 10 lines of code. I try to keep them small and focused. There's even a PR with 1 line change. Of course, some are larger, but they are very few.

  • Q: Maybe your code is really bad and he doesn't know how to address it?

    A: Since day 1 I showed everyone that not only do I welcome feedback on my code, but I actively ask for it. I've addressed every single comment on my PRs and when I'm not 100% sure about my approach... I do as others suggest. I've even changed my code even when I disagreed, just to show that I'm cooperative.

  • Q: Maybe he really doesn't have any time to review?

    A: Nope, I can see him working on objectively less-prioritized issues

  • Q: How can you be sure he is "intentionally" not merging your code?

    A: The most obvious example is a comment he left on one on my PRs. He stated that he looked at the code, it looks good, but it was low-priority feature and he felt that I should have spent my time on something more important. Even if that were true (which was not: the feature was on the high-priority list and only took me 15 minutes to implement) why wouldn't he merge the code?

Again... how do I deal with this?

I do not want to cause a toxic situation.

I would like to make him feel safe, I'm not a threat.

Contacting management is only a last resort solution for me.

5
  • 5
    Have you talked to him about this? How do you know his intentions are bad?
    – mcknz
    Feb 28, 2019 at 0:38
  • I can think of a dozen reasons why he wouldn't merge (without being toxic), but this question fails to say why he SHOULD merge.
    – Mars
    Feb 28, 2019 at 5:40
  • 4
    Also, you haven't explained why these pull requests are slowing you down. If there are changes that build off them, then you probably need to remind your senior that your current task is dependent on a task that is currently awaiting review
    – Mars
    Feb 28, 2019 at 5:44
  • Is timely merging of PRs actually part of your development process? Does only this colleague merge them? Does merging more PRs require QA resources down the line that you are maybe not aware of?
    – simbabque
    Feb 28, 2019 at 14:31
  • 1
    You can't know his intentions are "bad", period. Thinking you do will be harmful to your work relationships. Go ask him. "Hey, I notice my PRs are stacking up, and it's slowing me down. Is there something you'd like me to do differently?"
    – Peter
    Feb 28, 2019 at 15:09

4 Answers 4

15

To paraphrase Hanlon's Razor, never attribute to malice that which is adequately explained by incompetence, good intentions, or general lack of effort.

I don't see any evidence here that your colleague is "slowing you down intentionally," or otherwise engaging in sabotage. After all, this has only been in the last couple of days.

What does appear to be evident is, well, a failure to communicate.

You need to ask directly, without accusation, why your PRs aren't getting merged, on a case-by-case basis.

If you are given factually incorrect information, then you need to push back a little harder.

Also try to involve your colleague a little more in advance of the work you are doing, and schedule time to do in-person code reviews.

If you want to escalate this to management, you will need to document a history of behavior on his part that doesn't make sense, or is actively working against the good of the project.

You can only build that case by continuing to engage in dialog.

1
  • The Q&A added here: "How can you be sure he is "intentionally" not merging your code? A: The most obvious example is a comment he left on one on my PRs. He stated that he looked at the code, it looks good, but it was low-priority feature and he felt that I should have spent my time on something more important. Even if that were true (which was not: the feature was on the high-priority list and only took me 15 minutes to implement) why wouldn't he merge the code?" seems like a deliberate act to say the least.
    – Leon
    Feb 28, 2019 at 7:41
6

Bring this up with him as sort of a reminder, something along the lines of "I've noticed my recent PRs had stacked up lately, can you check them out if you have the time?".

Of course, document your work as well: document when you started, when you finished them, when you reminded your senior about them, and when they get reviewed and merged. You'll at least have something to show your actual performance and where the next step of the process falls short, something you had very little control over.

No need to jump to management here, you'll only make your relationship with the senior sour once he finds out. Management will get involved if the overall progress turns out be slow, and when they do, you'd have been able to document your work to show your actual performance.

2

A few questions from what you write here:

  • What do you mean by "found out that he is not merging it intentionally"?
  • How long is it actually taking? Minutes? Hours? Days? Weeks?
  • Does your ticketing system have time tracking?
  • Do you do daily/weekly stand ups/status reports?
  • Is there an established standard around time-to-review?

Comments:

Communication and conflation of responsibilities strike me as the issue here.

Doing a a lot of small changes with a PR for each can be more fatiguing and disruptive. If I got pinged by developers every 30 - 60 minutes to stop what I'm doing so I can review their thing, I'd be unhappy with the constant interruptions. It's also possible that they set aside time to batch out PRs, or timebox their review time to focus on their own tasks. All depends on what their metrics are. You example might be an isolated incident, or what they think is a "lesson", or whatever. Best to communicate with them about your feelings around it. You could always have responded with something like "Understood, I'll focus on higher priority issues first from now on. Other than that, is it okay to have this feature merged in?". Doesn't really matter where it was on the list in actuality. Their perception was that it was low priority, so just decide if this is a battle you want to fight.

As for your code being bad, I have no idea, but you thinking you're expressing being open to criticism and being perceived as being open can be two very different things. Best to communicate with a little 1-on-1 with the senior about how you're doing when it's convenient for them.

Objectively less prioritized for whom, and how are you seeing this? In general, it's good advice to mind you own business about the work others are doing.

Look, the ticketing system and PR commenting system are your best friends here. Make sure you're tracking when you start, when something is submitted for review, what is blocking what, etc (Jira offers all of these things). PRs are great places for getting feedback on your work, and a source of documentation around the quality of that work. Leave a comment on the PR with a friendly bump if it has been open for > 1 day.

As for the project and it's health, that's not really your domain here. You can't care more than management (or your senior developer) cares without going insane, or sitting in a spiral of frustration. Once the PR is submitted, it's his to deal with. If you've got stand ups, just call out that you're awaiting feedback on X, Y, Z. You're working on tickets A, B, C. That gives them an opportunity to speak to either of those things.

2

CYA. Document, document, document. Your documentation should stick to the facts:

  • $manager urgently asked for $feature on $date, was implemented on $date and handed to $senior for review/merge. Release of the next version was at $date, and I reminded $senior a week before that, but the feature was not made available to customers in that version.
  • $customer reported $bug on $date, was fixed on $date and handed to $senior. $customer followed up on $date, version available at that time did not contain fix because it was not yet merged. $customer sounded angry, so I reminded $senior on $date, merge was not done until $date.

Once you are asked, present to management your documentation. Then, shit may or may not hit the fan, but the fallout will not be on you.

You must log in to answer this question.