Pull Requests & Defensive Programming – It’s All About Trust

A very common thing I see on development teams is reliance on code reviews for every check-in (in this age of Git-Everything, often referred to as “Pull Requests”). This can create bottlenecks in the delivery process, as our peers are pulled away from their own work and we have to wait for their feedback. And, often, the end result is that these reviews are superficial at best, missing a tonne of problems while still holding up delivery.

Pull Request code reviews on a busy team

But why do we do these reviews in the first place?

I think of it in programming terms. Imagine a web service. It has a number of external clients that send it requests via the Web.

Some clients can be trusted, others not

These client apps were not written by us. We have no control over their code, and therefore can’t guarantee that the requests they’re sending will be valid. There’s a need, therefore, to validate these requests before acting on them.

This is what we call defensive programming, and in these situations where we cannot trust the client to call us right, it’s advisable.

Inside our firewall, our web service calls a microservice. Both services are controlled by us – that is, we’re writing both client and server code in this interaction.

Does the microservice need to validate those requests? Not if we can be trusted to write client code that obeys the contract.

In that case, a more appropriate style of programming might be Design By Contract. Clients are trusted to satisfy the preconditions of service calls before they call them: in short, if it ain’t valid, they don’t call, and the supplier doesn’t need to waste time – and code – checking the requests. That’s the client’s job.

Now let’s project these ideas on to code reviews. If a precondition of merging to the main branch is that your code satisfies certain code quality preconditions – test coverage, naming, simplicity etc – then we have two distinct potential situations:

  • The developer checking in can be trusted not to break those preconditions (e.g., they never check in code that doesn’t have passing tests)
  • The developer can’t be trusted not to break them

In an open source code base, we have a situation where potentially anyone can attempt to contribute. The originators of that code base – e.g., Linus – have no control over who tries to push changes to the main branch. So he defends the code base – somewhat over-enthusiastically, perhaps – from bad inputs like our web service defends our system from bad requests.

In a closed-source situation, where the contributors have been chosen and we can exercise some control over who can attempt to check in changes, a different situation may arise. Theoretically, we hired these developers because we believe we can trust them.

I personally do not check in code that doesn’t have good, fast-running, passing automated tests. I personally do not check in spaghetti code (unless it’s for a workshop on refactoring spaghetti code). If we agree what the standards are for our code, I will endeavour not to break them. I may also use tools to help me keep my code clean pre-check-in. I’m the web service talking to the microservice in that diagram. I’m a trusted client.

But not all developers can be trusted not to break the team’s coding standards. And that’s the problem we need to be addressing here. Ask not so much “How do we do Pull Requests?”, but rather “Why do we need to do Pull Requests?” There are some underlying issues about skills and about trust.

Pull Requests are a symptom, not a solution.

Leading By Example

We’re used to the idea of leaders saying “Do as I say, not as I do”. Politicians, for example, are notorious for their double standards.

But the long-term effect of leaders not being seen to “eat their own dog food” is that it undermines the faith of those being led in their leaders and in their policies.

When we see public servants avoiding tax, we assume “Well, if it’s good enough for them…”, and before you know it you’ve got industrial-scale tax avoidance going on.

When we see government advisors breaking their own lockdown rules, we think “Actually, I quite fancy a trip to Barnard’s Castle”, and before you know it, lockdown has broken down.

When we see self-proclaimed socialists sending their children to private schools, we think “Obviously, state schools must be a bit crap” and start ordering prospectuses for Eton and Harrow.

And when we see lead developers who don’t follow their own advice, we naturally assume that the advice doesn’t apply to any of us.

If you want your team to write tests first, write your tests first. If you want your team to merge to trunk frequently, merge to trunk frequently. If you want your team to be kind in code reviews, be kind in your code reviews.

As Gandhi once put it: be the change you want to see in the world. Be the developer you want to see on your team.

This means that, among the many qualities that make a good lead developer, the willingness to roll up your sleeves and lead from the front is essential. Teams can see when the rules you impose on them don’t seem to apply to you, and it undermines those rules and your authority. That authority has to be earned.

This is why I’ve made damn sure that every single idea people learn on a Codemanship course – even the more “out there” ideas – is something I’ve applied successfully on real teams, and why I demonstrate rather than just present those ideas whenever possible. You can make any old nonsense seem viable on a PowerPoint slide.

As a trainer and mentor – and mentoring is a large part of leading a development team – I choose to lead by example because, after 3 decades working with developers, I’ve found that to be most effective. Don’t tell them. Show them.

This puts an onus on lead developers to do the legwork when new ideas and unfamiliar technologies need to be explored. If you need to get your legacy COBOL programmers writing unit tests, then it’s time to learn some COBOL and write some COBOL unit tests. This is another kind of leading by example. Getting out of your comfort zone can serve as an example for teams who are maybe just a little too comfortable in theirs.

And this extends beyond programming languages and technical practices. If you believe your team need a better work-life balance, don’t mandate a better work-life balances and then stay at your desk until 8pm every day. Go home at 5:30pm. Show them it’s fine to do that. Show them it’s fine to learn new skills during office hours. Show them it’s fine to switch your phone off and not check your emails when you’re on holiday. Show them that you don’t marginalise people on your team because of, say, their gender or ethnic background. Show them that you don’t act inappropriately at the Christmas party. Show them that you actively consider questions of ethics in your work.

Whatever it is you want the team to be, that’s what you need to be, because there are far too many people saying and not doing in this world.

Are Only 1 in 4 Developers Trusted?

Following on from my previous post about Trust & Agility, I ran a straw poll on the @codemanship Twitter account as a sort of litmus test of how much trust organisations put in their developers.

A new monitor is a decision of the same order of magnitude of risk of the proverbial “$100 decision” that I’ve used for many years now when assessing the capacity for trust – and therefore for decentralised decision-making – that agility requires.

If I wanted a new monitor for my home office, that would be entirely my decision. There was, of course, a time when it would not have been. That was when I was a child. If 13-year-old me wanted a new monitor, I would have had to refer that decision up to my parents.

When I got jobs, I made my own money and bought my own monitors. That’s called “being a grown-up”.

I’ve worked in organisations where even junior developers were handed corporate credit cards – with monthly limits – so they could “just go get it”, and not bother management with insignificant details. Smaller decisions get made when they need to be made, and they don’t get caught in a bottleneck. It really works.

So why are 3 out of 4 developers still not trusted to be grown-ups?

UPDATE:

A few people have got in touch giving me the impression that they took this to be literally about buying monitors or about company credit cards. These are just simple examples. They indicate a wider problem often. Developers who I’ve seen wait weeks for a monitor, or a laptop, or a software license, have tended to be the ones who “wanted to do TDD but weren’t allowed”, or who “wanted 20% but the boss said ‘no’ “, or who wanted to go to a conference but it was denied. These tend to be the teams that can’t self-organise to get things done. In my 20 years’ experience of Agile Software Development, the correlation has been very close between teams of developers who have to ask permission for a new monitor – just as an example (IT’S NOT ABOUT BUYING MONITORS!) – and teams who have to ask for permission to automate deployments or to talk to the end users directly.