Programming

PR Reviews: not the messiah, not a naughty boy

On Tech Twitter there seems to be a lot of chat recently about how the Github Pull Request (PR) review process is “broken” or underserves internal teams (an example) that have strong collaboration practices such as pairing or mob coding.

Another objection raised is the idea of internal gatekeeping within development groups, I’m not sure I fully follow the argument but I think it runs along the lines that the PR review process allows powerful, influential members of the group to enforce their views over the others.

This is definitely a problem but frankly writing AST tools linked to the “merge to master” checks is probably a more controlling tool than spending your time policing PRs.

With open source projects often all the action happens at the pull request because the contributors often have no other interaction. Proponents are right to point out that if this works for open source projects do you really need to put effort into other practices upstream of the PR? Opponents are also right to point out that adopting the work practice of an entirely different context into your salaried, work context is crazy. They are both right, individual organisations need to make deliberate decisions using the challenges of both side to the way they are working.

I’m not sure how much of this is a fightback by pairing advocates (that’s how I interpret this thread). There is a general feeling that pairing as a practice has declined.

In my work the practice is optional. As a manager I’ve always known that in terms of output delivery (which before people object, may be important particularly if you’re dealing with runway to meet salary) pairing is best when you’re increasing the average rather than facilitating the experienced.

I think even with pairing you’d want to do a code review step. Pairs are maybe better at avoid getting into weird approaches to solutions than an individual but they aren’t magical and if you are worried about dominant views and personalities pairing definitely doesn’t help solve that.

So I’d like to stick up for a few virtues of the Pull Request Review without making the argument that Pull Requests are all you need in your delivery process.

As an administrator of policy who often gets asked about Software Development LifeCycles (SDLC) as a required part of good software governance. It is handy to have a well documented, automated review process before code goes into production. It ticks a box at minimum disruption and there isn’t an alternative world where you’re just streaming changes into production on the basis of automated testing and production observability anyway.

As a maintainer of a codebase I rely on PRs a lot as documentation. Typically in support you handle issues on much more software than you have personally created. Pairing or mob programming isn’t going to work in terms of allowing me to support a wide ranging codebase. Instead it’s going to create demand for Level 3 support.

Well-structured PRs often allow me to understand how errors or behaviour in production relate to changes in code and record the intent of the people making the change. It makes it easier to see situations that were unanticipated or people’s conception of the software in use and how that varies from actual use.

PR review is also a chance for people outside the core developers to see what is happening and learn and contribute outside of their day to day work. Many eyes is not perfect but it is a real thing and people who praise teaching or mentoring as a way to improve technique and knowledge should be able to see that answering review questions (in a suitable form of genuine inquiry) is part of the same idea.

PRs also form a handy trigger for automated review and processes. Simple things like spelling checks allow a wider range of people to contribute to codebases fearlessly. Sure you don’t need a PR to use these tools but in my experience they seem more effective with the use of a stopping point for consideration and review of the work done to date.

Like a lot of things in the fashion-orientated, pendulum swinging world of software development good things are abandoned when no longer novel and are exaggerated to the point that they become harmful. It’s not a world known for thoughtful reflection and consideration. But saying that pull request reviews undermine trust and cohesion in teams or a formulaic practice without underlying benefit seems unhelpfully controversial and doctrinal.

Standard
Work

Against Trust

Work is generally better in high-trust environments, it is one of those success breeding success things. However you cannot simply get to high-trust because it is mutually beneficial to both parties. During the trust gaining stage things must be proven.

One of the nice things about working in IT (rather like studying for a Maths degree) is that things don’t have to be taken on trust. Sometimes it is convenient to but if needed to you can often make the effort to show that some thing happens exactly as you said it does. That’s the joy of unit-testing for example. Scala language advocates have also talked about the determinate nature of its static code checking. The code has to be right, otherwise it wouldn’t compile.

So in general I like trust, because its productive; but I don’t like to trust because I don’t feel I should have to. It should always be possible to show that what you are saying is true. Of course in consultancy that kind of proof is hard to come by so instead of absolute proof you instead have to come up with metrics that you can measure to allow you to fail fast instead of wasting a lot of time (and trust!) on an idea that isn’t going to pan out. This means you do not have to ask people to trust your ideas, merely agree some criteria for judging success and failure at minimum cost.

These ideas are some touchstones of how I have come to see effective IT within organisations. They are also what I feel informs me in a recent debate going through ThoughtWorks about Pay Transparency. You can find an effective starting point in Matt Buckland’s blog and what I think is an excellent statement of why Pay Transparency is a good idea in Francisco’s blog.

For me trust is good thing but should never be a necessary thing. Matt’s final point in his post is the most important as I think it is what really informs his view on pay transparency:

In all of these discussions the culture of transparency is held as the ultimate goal of an organisation and I’d wonder if this is the case. In my opinion it is a culture of trust we should strive for. Employees who feel they are paid fairly, because they have effectively sold the “exploitation of their labour” with full knowledge of the market in which that labour is sold will be better able to realise their own position … but instead trust they are given a fair salary based on their own personal circumstance and that the company they work for will aid them in their strive to grow and develop as an individual – salary, the nuts and bolts measure of their value, will become secondary.

From my point of view I want a culture of transparency because I believe that from it a culture of trust emerges. I don’t want to have to trust anyone, I would rather they proved what they say. In exactly the same way as I have to build trust with clients. In exactly the same way as a client perhaps trusts my code but can inspect my test reports at any time.

In my experience the processes and procedures around pay in the industry have been exceedingly opaque. There are so many variables and factors that pay is effectively arbitrary. I have often thought that pay really only reflects the ability of an organisation to function effectively without you.

In this context salary transparency is really the most important thing you can do to build trust around pay as few organisations can articulate the process behind salaries except that it is down to the abilities of individuals to negotiate for themselves and devil take the hindmost.

This post title says “Against Trust”, but it is really “Against Blind Trust” which is what people who reject Pay Transparency but forward no alternative mechanism are really asking for.

Standard