My post from a year ago: “Some observations on preference for Pull Request“

Table of Contents

What PRs enabled

Deep work (“flow”)

As there is no one to bounce ideas off, or to immediately validate or disagree with my own ideas, I think deep and hard into a problem. I consider solutions and tradeoffs, and get into a flow state and get the work done without interruptions, which is deeply satisfying and arguably why people become software developers in the first place.

In my opinion, this needs a few prerequisites: good documentation and running on a makers schedule.

On the other hand, I am aware that I am at least mid-level engineer where I have seen enough code of similar nature to spot patterns and have mild to strong opinions on certain things. I am unsure whether this works well or relevant for junior software engineers.

Some flexibility in how I work

In our tech induction at Octopus Energy, it was explicitly stated that (paraphrased) “we care about shipping high-quality software, maintainable code and [insert other things about writing good software]; what we don’t care about is how long you sit in front of the keyboard for”.

This meant that some days I took a walk on a sunny day at 10am to think about a problem; some days I took time to read Pythonic Programming when I needed a break; some days I worked until 8pm because I was in the flow. It also probably no longer mattered whether I was in the same time zone as everyone else, because I no longer need synchronous communication to do my job well.

It also meant I could wake up everyday and look forward to code.

Skills to develop for PR-based culture

Get feedback early (from the right people?)

I have definitely spent a day to complete a task, only to find out that the code change was not where it was meant to be, and I had to rework how to complete the task.

While it took “only” one day to implement the task, and I figured it could not have taken me faster, it could still feel a whole day “wasted”. Not for me, because I learnt something and also my general mentality is that change is the only constant. But I feel that this does impact some others more.

This is not to say that this definitely would not happen in pair programming culture, but it would more likely than not be caught by my pair, or it won’t be discovered until much later if it was indeed a problem.

For bigger tasks, getting feedback early enough is also a key skill to develop to avoid what happened above.

Being just good-enough

Perhaps this depends on character, but I find myself obsessing about how perfect my commit history is and my variable and method names. These, I know, if I paired with someone else, might be good enough to pass.

Quality code reviews

For the first few PRs that I reviewed, I have definitely fallen into the trap of commenting at typos in comments. Thus, I find this chart helpful to start developing the skill of doing meaningful code reviews: The Code Review pyramid

What Pair programming and PR-based culture both enabled

Discussions, context-building and knowledge sharing

In my previous post, I have assumed that because of live discussions, we as programmers “slowed down to discuss why we were implementing a feature”. However when I work alone, I have also had the chance to do so – again importantly enabled by the prerequisite of having documentations.

In fact, arguably I have gotten more out of the PR-based culture because more likely than not, there had been enough discussions in the past that I could find, and thus discussions were no longer simply between programmers who pair, or the team composition at that time, but in other teams and time too.

Code quality

My heurestics on code quality as an Individual Contributor: 1. How much yak shaving I need to do to complete my task at hand, 2. How much WTF I say when I look at the codebase.

I can confirm that these are unrelated to whether codebase is developed from pair programming or PR-based culture.

Prerequisites for good PR-based culture

I can directly correlate why I enjoy PR-based culture so far with these prerequisites.

Documentations (formal and historic discussions)!!

We have up-to-date (whoa!) formal documentations and many historic discussions around problems similar to the one at hand available, as old PRs (descriptions and discussions), Slack threads and commit messages.

We get regular reminders that if a discussion or a question, no matter how trivial or stupid they might seem, could remotely be known as knowledge shraing, we have to post them in public channels. I definitely have had my question in a direct message moved to a public channel.

In fact, there are so much of them, in the first few weeks of my onboarding, I search for answers on Slack first before reaching to StackOverflow.

I acknowledge that there is a huge amount of work that went into this. For example I could find a commit message from a few years ago, whack the commit subject on Slack and find the relevant PR link and thus the context of the PR and any discussions – well, someone has to get the GitHub/Slack integration setup effectively!

Conventions and Linting

We have a bunch of conventions documented and automated linting rules set up to keep the code standards and thus its quality.

Again, I acknowledge that there is a huge amount of work that went into this. Someone has to think about these things, get alignment from everyone, facilitate discussions, and then get it written down or codified.

Run on a maker schedule

I now have weekly catch ups instead of daily. Meetings are rarely longer than 30 minutes. Often the only entry on my calendar for the day is “Lunch”.

Addendum – did the points actually matter?

Did the points I raised from my 1-year old post “Some observations on preference for Pull Requests” actually matter?

“Maintain a cleaner git history” and “Easier to see change set in PR”

They are a merely a side effect of PR reviews. In other words, in order to make PR reviews easier, having clean git history and thus reviewing the change set in atomic commits are important.

I still don’t see how they actually matter in software development, so I am going to attribute these merely as habits of those switching from PR-based culture to Trunk-based development cultures, and should not be strong arguments for PRs.

“Identifying the exact problematic commit”

In reverse, I would say this is a trunk-based development mentality that was brought to PR-based culture. PRs run the entire CI before they are merged, so that we already gained certain amount of confidence before the commit is on the trunk. Whereas in trunk-based development, we do not know whether a commit, pushed directly to trunk, will break the CI or not.