"team lead" acting as gatekeeper

Question

So I started a new (software development) job a few months back, and they use a "scrum-inspired" means of management. One of the rules is that the team lead for each team must sign off on and personally merge every pull request to main, irrespective of other team members' / other teams' reviews. (Note that this isn't necessarily a full code review; just a check that the code fits the requirements and other standards.)

This is onerous at best, but is made worse by the fact that my team's lead claims to be very busy with meetings (which I don't doubt) and regularly "forgets" to do the reviews in a timely manner. I've had to apologize to product for having a feature miss a (biweekly) release cut twice now, and my "personal record" is 8 PRs lined up for him.

I often bring up "waiting for code review" as a blocking issue during the daily scrum meeting. I've been leaving it at that rather than "I'm not getting software shipped because you, yes you, won't do your job", of course, but I could say it in a more assertive way.

This procedure isn't his policy; it comes from management and all (5 or so) teams follow it. I have some sympathy for him because it does appear to be a lot of work (the team's size is pushing the limits of what I would be willing to manage), but at the end of the day it's his responsibility, for better or worse.

What can I do to make myself and the team more efficient without jeopardizing our relationship?

Answer 1

Unfortunately, there's probably not a whole lot that you can do to remedy this situation, especially as a new hire outside of a management role. However, I can give you a few suggestions.

First, don't apologize for your work missing the release cut when it's not missed because of your fault. It doesn't sound like there's an agreement for how long it should take for the "Scrum Master" to complete this review. As long as you are submitting your work to enter this review gate in a timely fashion, you shouldn't be thinking that you're doing anything wrong.

Second, use the Sprint Retrospective. If your organization is truly agile (which they may not be, based on your description), then there should be opportunities to reflect on the way of working on a regular basis. Keep bringing up the problems with this process - the long queues of reviews, the missed releases. There are probably other problems with this, around getting work integrated and delivered, so any time not having reviews handled in a timely manner affects you, bring it up and be specific about the impact.

You may also want to talk to your manager to make sure that your performance is not being negatively viewed because of someone else's constraints. If it is, you may want to find a way to get ahead of this so the quality and timelines of your work are clear.

Answer 2

There are going to be a few scrum-aware answers here, which is great, but I'm going to come at it from a more... traditional perspective.

If you suspect there is something that is holding the team back, it would be appropriate to discuss it with your manager. It doesn't have to be overly formal, or dramatically accusatory, you just indicate that you think there is a systematic problem.

If this person is your manager, the conversation is a bit awkward, but it should still occur. But once you are certain they are aware of the problem, it really isn't your problem to deal with anymore. Just ensure you are as productive as possible.

I actually don't think inherently improper for one person to be the gatekeeper of a repository (they may be a SME for example), though there are obvious concerns around bus factor. So I am certainly willing to assume there may be good reasons why things they are the way they are.

I agree with Joe that you should not apologise for things that are out of your control, and for the record, I am a bit proponent for apologising when you do make mistakes.

However, you have a bit of an obligation to ensure your tickets get merged in a timely manner, as much as you can.

So, as you approach the deadline, you need to make things as visibile as possible for the blocker. This may involve sending them a instant message with a prioritised list of outstanding reviews. Example:

Hey Bob. I have a few things I need merged. Including some stuff that needed to be merged last week. I've grouped things by when they are due, so hopefully you can merge these ASAP.

TICKET OVERDUE

• [PR-123] Add important feature for customer blah
• [PR-234] Add some other feature

TICKET DUE FRIDAY

• [PR-345] Make widget darker

TICKET DUE NEXT FRIDAY

• [PR-456] Fix speiling

You need to make it as easy as possible for them to target the things that are immediately important, as well as highlighting problems. Also by using days of the week names, rather than dates, it's easier to understand the due dates when the message is received.

Obviously if things get worse and worse, more and more things are going to slip into the overdue category, and that looks more and more obviously a problem.

And lastly, I just was to say, if this is a systematic problem which is highlighted all the time, you really just need to try to make your stuff as easy to merge as possible. Because you don't want the situation where other people's stuff gets merged and yours doesn't because it's harder to review.

Answer 3

There isn't a dichotomy between doing nothing and being accusatory. My suggestion would be to raise the issue as an issue, not as an accusation. You suggested:

"I'm not getting software shipped because you, yes you, won't do your job"

Don't do this, obviously (you know this already). Rather, you can say something like this:

I believe that our system whereby you have to approve every single PR before it gets merged is not efficient. You're a very busy guy, and you can't spend all your time approving PRs. As a result, we have backlogs of features that aren't getting merged and deployed to production on time. Can we consider revisiting this requirement in order to improve efficiency?

This isn't avoiding the problem (as you seem to be doing now) nor is it being accusatory and a personal attack, as your suggested alternative. It's raising the issue as a process problem, rather than a person problem and asking if that process problem can be resolved. It's also acknowledging that simply telling your manager to work harder (obviously a bad idea) is not going to work (for many reasons, but primarily because he's already working hard).

Answer 4

In the situation you describe, the problem is not that your team lead is acting as a gatekeeper, it's that the team lead is forced to be a gatekeeper, and is running out of bandwidth.

The problem is the process, not the team lead, and the solution is to fix the process, not the team lead -- so there's no reason even to think of being accusatory.

In your daily standups, you should mention it when you are blocked waiting for PRs, or even when waiting for PRs is slowing down your work by making you juggle multiple branches, etc.

At sprint retrospectives, you should bring up the matter again and try to help out with solutions. One thing you might suggest, for example, is to add a second line of reviewers that can pick up and approve the more trivial PRs, escalating to the team lead if there is anything they aren't sure about.

Answer 5

You shouldn't apologize for a feature missing a deadline due to this internal process. Your work ends once you submit the code for review/merge. From your perspective, it's submitted and "done" at that point since you have no authority or control after that point. If someone asks why a feature missed the cut, you can tell them "I completed the work for that feature and submitted the changes, but it wasn't included in this release for some reason". Point them towards your team lead for more information. This does two things. It lets the other person know that the work has indeed been completed, and it gives your leader a chance to explain what happened. Maybe he didn't have time to process the PR, or maybe there's some other reason that you're not aware of. The goal here isn't to blame or embarrass your leader. The goal is to change the conversation from "you didn't get this done on time" to "these procedural requirements make it difficult to complete work on time". You are more likely to see procedural changes when more people are aware of the problem.

To make living with these requirements easier, one thing your team can do is to implement a code freeze at some point prior to the deadline. Any changes submitted prior to the code freeze will be included in the release, and anything submitted afterwards waits for the next release. This freeze can be time based (e.g., 3 days before the deadline), or adjusted based on the size of the backlog. Ultimately, the idea is that the time period between the freeze and the deadline is long enough to process all of the eligible PRs that are backed up in the queue. That way, there's a phase of the development cycle where your team lead is almost solely responsible for making progress. They can block off that time on their calendars, decline meetings, and otherwise ensure that they have nothing else distracting them from doing what they need to do. When PRs trickle in unpredictably, it's hard to prioritize them and easy to shrug them off for "later". Others may complain about your team lead's unavailability during that time but again, remind them that it's a side effect of the policies that your team is required to operate under.

It might also help to have an informal chat with some of the folks on the other software teams to see if they're doing things differently than your group. You implied that your group was larger than the others, but there could be other factors at play here. Sharing best practices between teams is a great way to get new, already-tested ideas.

Answer 6

Other answers are good around leveraging leadership and agile ceremonies like retro to expose the constraint. Two tactical things you could do on top of that:

1. Assuming you use a tool such as JIRA or Azure DevOps to manage the process, create a column/status for "waiting for PR merge". It will be clear to the scrum master what they need to accomplish during that day. It will demonstrate to leadership why items are stuck and/or then they can choose to manage it.
2. If that's already in place, can you consider a different git flow-like model where all the engineers work off develop (branched from main) and you collectively at end of the week create a single PR to main? I concede this requires maintenance from the team but it shouldn't be too much churn and relieves the constant dependency on the Scrum Master.

This larger gatekeeping was likely put in place because someone merged a feature in that wasn't ready yet and management added process to protect it. This may be a hard wall to tear down. Suggestion 2 helps your scrum team to do this and could demonstrate a new way all teams work in the organisation to manage this concern.

Answer 7

As a lead who has unfortunately been in this PR gatekeeper position before I will give you a slightly different perspective on this. Because while you might not be able to change the process, there are things you can do to help.

First, understand the source of this. It is very likely that this process exists because at some point in time "the team" did a bad job at code reviews. This happens a lot because when no one is on the hook, then no one feels directly responsible. It's a kind of Bystander effect This can sometimes result in PR wars or the opposite a parade of "LGTM" reviews. Neither is good.

Second, I want to say that, there have been a lot of really good answers on how to approach this on a systemic basis in your development process, retrospectives, one-on-one communication, putting it into blocked. All good ideas. I would caution to not make it personal or point fingers at individuals or be passive-aggressive about it. A major part of being on a team is having empathy for everyone. And that brings me to your lead...

Leads spend a lot of time in meetings, on planning, both on the architectural level and people level. After all, this, pull requests often come into this as a low priority. Understanding this could lead you to help understand how your and other developer actions can affect this process. Anything you can do to make my life easier when I am doing code reviews will speed up the process.

• Keep your pull request up to date with the main branch. Check it and sync it several times a day. At least in the morning and sometime in the afternoon. Nothing is more annoying than having to sync a pull request prior to doing a review, hoping that there will be no conflicts. (and that leads to...)
• Resolve your conflicts. Make sure you are resolving any conflicts that your branch might have with the main branch as soon as you can. If I see a conflict I might take the time to resolve it, or if I don't have the time I'm going to kick it back to you.
• Make sure PR pipeline is green. If you have automated tests or static analysis in your PR process make sure they are all passing. That often means that after you have submitted the PR you need to check on it a few minutes later to make sure all lights are green. I cannot tell you the number of times I've come across PRs that I am ready to review but I stop because tests or static analysis is red or yellow.
• Write great unit tests. You should be doing this anyway but if I see unit tests in a PR, and the pipeline is passing all the unit tests, that is a good sign. Those tests are going to be the first thing I look at in the PR. Do they look correct? Are they testing the right things? Are both positive and negative cases covered? Good passing tests give me confidence in the rest of your code.
• Don't keep your PRs atomic. This goes against conventional wisdom but so does this entire process. Most devs have been taught that one issue -> one branch -> one PR. However, you need to know sometimes it is a good idea to break from this. If I'm reviewing PRs I would much rather review 1 medium size PR than 5 or 6 tiny PRs that all play in the same area of the code. In particular, if they touch the same code, as a reviewer I start to get concerned about interactions between these changes. I have often kicked these back to the developer or developers and asked them to merge the changes into one branch for a combined PR. More than once this has then caught conflicts or issues that otherwise might not have been seen. Remember devs don't own branches, you can share them.
• Leave a great pull request comment. Introduce the change, tell me what you did and why. Don't just tell me the ticket or tickets this is related to, provide me a link to them. Include the important bits of the ticket in the PR comment. Tell me the importance of this PR and the dependencies. Just telling me it is due for the sprint or by Friday is not helpful. I know this. Everything is due Friday. If it is a UI change, include some screenshots.
• Leave great comments in your code. Commenting in the code not only helps with the current PR but also all future developers. All code is "self-documenting" when you are writing it because you have it all in your head. Imagine that you are a new developer seeing this code in a year knowing nothing about it. Always use comments to explain the why.
• Feedback your own PR. Nothing prevents you from reviewing your own PR and leaving comments. If there is something that you think I might look at with greater scrutiny then you can preemptively comment on that line. If I see you've commented on it then maybe I won't have to ask you about it. The one caution here is that if you feel the need to do this first ask yourself if you should just put the comment in the code itself. (see Leave great comments in your code.)
• Learn from previous PRs. If I have reviewed your code before and left comments on what needs to be changed. Don't submit code that does the same thing again. At best I'll miss it and we will have code that doesn't match the rest of the codebase. At worse, I'll just flag it again and stall your PR. Additionally, now I'm going to start looking at your code even closer.
• Don't be clever. "Clever" code is fun to write but a pain in the ass to review. Write simple and clear code using constructs and conventions that are common to your language and your code base. Don't submit brain teasers in your pull requests. I might waste my time trying to figure out how it works or I might just send it back to you. Even if I do understand it, I'm likely going to send it back to you to simplify it.
• Be a great reviewer yourself. Assuming the entire team can review code then you can help with all of the above on everyone's pull requests. If I jump into a PR and see that everyone else has taken an effort to review it (and not just a bunch of "LGTM" comments) then I feel more confident about the process and I need to spend less time to review it. If you see two PRs that could be merged into one, talk to the other dev and merge them. If you see other PRs sitting with unresolved conflicts or broken pipelines, fix them or have the original dev fix them. If you see a PR lacking a good comment, flag it and talk to the dev. If you see a PR with code similar to code that has been rejected before, comment on it.

This last bullet point I cannot stress enough. There are several benefits to this. First, I will start to trust your code more which means less scrutiny on your PRs. Second, If you ever want to get to a state where the team is doing code reviews as a team and not with a gatekeeper then the team needs to demonstrate that it can do effective PR reviews. All of this is necessary for that to occur. Last, you will have evidence that the team is ready to change the process when the gatekeeper is able to primarily leave "LGTM" comments because the team has caught everything that might be an issue.

Answer 8

The Scrum Guide says:

Scrum Teams are cross-functional, meaning the members have all the skills necessary to create value each Sprint. They are also self-managing, meaning they internally decide who does what, when, and how.

If that were true for your team, your team would be held accountable for the missed deadlines, and reflect on how to improve process during the next retrospective, and implement the necessary process changes.

However, management has not given your team the authority to make the obvious change (have pull requests merged by more than one person), so you'll likely need to involve whoever makes such policy decisions at your organization.

In organizations, whoever makes a decision should be accountable for that decision. If management makes a decision, and you're held accountable ("have to apologize to product"), this principle is violated. To fix this, you should make transparent what caused the delay, so the correct person is held accountable, and is thus aware of and motivated to fix the process failure.

How to do this depends on how your organization is organized. In an agile org, the team is held accountable for work outcomes, so the team would apologize, and then change its process.

In a hierarchical organization, the team lead is accountable, so the team lead should apologize to product (and be more prompt about reviews in the future) or inform product that policy X is responsible, to allow an escalation to the proper person. In either case, you should not apologize, because you can't fix the problem. Don't take responsibility for things you can not fix.

As an aside to managers, this is why mixing hierarchical decision making with agile-style accountability is problematic. If you mix agile and hierarchical management, it's possible that people will have a different understanding of where one ends and the other begins, and it's thus very easy to end up with a model where decision rights and accountability are not aligned.

But back to you: Don't take responsibility for things you can not fix, but direct people to those who can. For instance like this:

Yeah, that sucked. The problem is that, according to policy, Joe is the only person allowed to merge changes, and he didn't get around to it. I hear he's been very busy with X ...

if you like Joe, you can also give him a heads up that product was pissed.

And if you see something headed for disaster, warn the person who can fix it:

Hey Joe, have you seen my PR for issue #1875? According to JIRA, it's still unmerged, but scheduled for the current sprint ...?

or

Hey Joe, have you seen my PR for issue #1875? I need your feedback by ... if we are to deliver it this Sprint.

(and if that person has a habit of throwing you under bus, you may wish to proactively inform the person who will be affected about the handover of responsibility, so they can follow up as needed)

Answer 9

Before you escalate into a potential confrontation with the team-lead, ask peers on your team if they have the same issue with late reviews. Maybe they have some tips for getting the team-lead's attention when it's important, maybe having to wait a while on reviews is "normal"/"acceptable" in your company, or maybe they also have the same issue and you can escalate together, increasing your credibility and minimizing your individual risk.

Answer 10

I'm gonna come at this from another direction and say the biggest problem is not your company policy; it's your branching system. It sounds like your team tends to have pull requests into Main which is built into your release packages, and the volume of PRs is exacerbating your manager shortage.

Ideally Version control branches should go some manner of {Feature} → {Develop} → {Release} but some common branch types are:

• Feature: A branch for working on a specific thing, ideally tied to a task on the board. No PR needed
• Epic/MajorFeature: Ties to an Epic instead of a task. PR should include subject experts or other devs working the same epic
• Develop: The communal starting branch. PR includes the whole team.
• Staging: For when the team isn't allowed to merge to release without some extra steps. PR should be the whole team and Product Owner
• Release: The Immutable Branch ready for Release.

At your next standup, ask your teammates if you can start using more interim Branches to consolidate your work. The larger PRs will look intimidating but will be easier to review than a pile of pulls. It also creates a review and retrospective cycle. Talk with your manager and find out where he wants to come into this. They might say they want to approve everything going into develop and deal with Major Feature PRs, or maybe just the total review at the end. If Major Features are independent of each other it may be best not to merge them until they are Validated by your Product owner/Customer at the end of sprint.

As for the scrum side of things, you've fallen into a fairly common scrum trap, your scrum manager has become more like a product owner. So somebody has to step up and do more of the scrum management. If nobody else is scheduling backlog grooming, reviews, or retrospectives then you just schedule them.

Scrum is for the team and Dark Scrum is for management, effective Scrum Leaders are slowly pushed into management which leads to Dark Scrum if no one steps up to take over.